Skip to content

Conversation

@SolidWallOfCode
Copy link
Member

With commit d07a6ac for #9474 and g++ 12.2.2 on Fedora 36 I get compiler errrors, which is basically this repeatedly.

In file included from ink_inet.cc:30:
In destructor 'virtual ats::CryptoContext::~CryptoContext()',
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'virtual ats::CryptoContext::~CryptoContext()' at ../../include/tscore/CryptoHash.h:171:86,
    inlined from 'uint32_t ats_ip_hash(const sockaddr*)' at ink_inet.cc:381:5:
../../include/tscore/CryptoHash.h:171:90: error: array subscript 'ats::CryptoContext[0]' is partly outside array bounds of 'ats::CryptoContext [1]' [-Werror=array-bounds]
  171 |   ~CryptoContext() { reinterpret_cast<CryptoContextBase *>(_base)->~CryptoContextBase(); }
      |                                                                                          ^
ink_inet.cc: In function 'uint32_t ats_ip_hash(const sockaddr*)':
ink_inet.cc:381:19: note: at offset 144 into object '<anonymous>' of size 272
  381 |     CryptoContext().hash_immediate(hash, ats_ip_addr8_cast(addr), TS_IP6_SIZE);
      |                   

My opinion is this is a compiler issue, that it's getting confused because the wrapper type is a subclass of the cast type. This causes repeated inlining of the same code, while array bounds of 'ats::CryptoContext [1] makes no sense at all, given there is no array of that type.

This fixes the problem by having CryptoContext not inherit from the protocol class. That inheritance is unnecessary and adds an extra virtual function call for no benefit. At that point it seems better to move the protocol class inside CryptoContext for better scoping.

@SolidWallOfCode SolidWallOfCode added this to the 10.0.0 milestone Mar 15, 2023
@SolidWallOfCode SolidWallOfCode requested a review from maskit March 15, 2023 14:29
@SolidWallOfCode SolidWallOfCode self-assigned this Mar 15, 2023
Copy link
Member

@maskit maskit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. The class inheritance didn't make sense. Bryan and I were aware of it but we just left it as it is.

@SolidWallOfCode SolidWallOfCode merged commit dac7b25 into apache:master Mar 15, 2023
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* commit 'c54a2e2b77151869ff014fbdc4c82cec0afcbb8c': (37 commits)
  Slight performance improvements before calling APIHooks::clear (apache#9480)
  libswoc: Update to 1.4.5 (apache#9522)
  CryptoContext: Clean up to avoid compiler problem. (apache#9521)
  Add TLSCertSwitchSupport (apache#9322)
  Add clang-format-tests to clang-format target (apache#9456)
  Adds the AR env variable to config.nice (apache#9515)
  Fix .asf.yaml (apache#9519)
  Hugepage config cleanup (apache#9479)
  Separate io_uring into a separate library. AIO in io_uring mode uses new io_uring lib. (apache#9462)
  Avoid memory allocation in CryptoHash (apache#9474)
  UnitParser: add unit parser support. (apache#9485)
  autest - Minor fix on the verifier_client test ext to allow setting only the http3 ports. (apache#9517)
  Remove support for port event polling (apache#9476)
  QUIC: Add support to configure UDP max payload limit. (apache#9486)
  Reduce the size of the APIHooks, eliminating enum gap (apache#9509)
  Add support for CMCD-Request header nor field to prefetch plugin (apache#9232)
  Eliminates padding from some common structs (apache#9481)
  Enable external file loading for sni.yaml. (apache#9501)
  Remove inactive include of IpMapConf.h (apache#9512)
  Cleanup: Remove RecModeT from the code. (apache#9487)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants