commit 709ba6eef8544cb806350a8e1ef0816aece45a10
author Adam Langley <agl@imperialviolet.org> Thu Aug 08 20:46:48 2024 +0000
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Aug 08 21:05:20 2024 +0000
tree 9a775a7d9bc31a75b389d85d59e7b41ca15f7be5
parent cee4fe2b78cfd2db8b4429b4fefd9831d3e63c04

Add virtual destructors.

b7f5443cfc added an abstract class without a virtual destructor, which
is fine in that context, but some places build with -Wnon-virtual-dtor
and thus:

  error: 'bssl::AesHwCipherScorer' has virtual functions but non-virtual destructor

Thus reflect this build configuration in our CMake build and add the
needed lines to satisfy the compiler.

Also put the classes in an anonymous namespace to more strongly hint, if
that is needed, that it can all be optimised down to almost nothing,
which it hopefully is.

Change-Id: Icd51782641073f55d1672dcf6a6adb82e3b27fb4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70087
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 26fad80..52fbceb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -169,7 +169,7 @@
   endif()
 
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_CXX_FLAGS} -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes")
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${C_CXX_FLAGS} -Wmissing-declarations")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${C_CXX_FLAGS} -Wmissing-declarations -Wnon-virtual-dtor")
 
   # In GCC, -Wmissing-declarations is the C++ spelling of -Wmissing-prototypes
   # and using the wrong one is an error. In Clang, -Wmissing-prototypes is the

ssl/s3_both.cc

diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
index 7db2730..b0e298c 100644
--- a/ssl/s3_both.cc
+++ b/ssl/s3_both.cc
@@ -659,11 +659,15 @@
   }
 }
 
+namespace {
+
 class CipherScorer {
  public:
   using Score = int;
   static constexpr Score kMinScore = 0;
 
+  virtual ~CipherScorer() = default;
+
   virtual Score Evaluate(const SSL_CIPHER *cipher) const = 0;
 };
 
@@ -673,6 +677,8 @@
  public:
   explicit AesHwCipherScorer(bool has_aes_hw) : aes_is_fine_(has_aes_hw) {}
 
+  virtual ~AesHwCipherScorer() override = default;
+
   Score Evaluate(const SSL_CIPHER *a) const override {
     return
         // Something is always preferable to nothing.
@@ -687,6 +693,9 @@
 
 // CNsaCipherScorer prefers AES-256-GCM over AES-128-GCM over anything else.
 class CNsaCipherScorer : public CipherScorer {
+ public:
+  virtual ~CNsaCipherScorer() override = default;
+
   Score Evaluate(const SSL_CIPHER *a) const override {
     if (a->id == TLS1_3_CK_AES_256_GCM_SHA384) {
       return 3;
@@ -697,6 +706,8 @@
   }
 };
 
+}
+
 bool ssl_tls13_cipher_meets_policy(uint16_t cipher_id,
                                    enum ssl_compliance_policy_t policy) {
   switch (policy) {