envoy icon indicating copy to clipboard operation
envoy copied to clipboard

Fix //test/common/common:bit_array_test on s390x

Open namrata-ibm opened this issue 1 year ago • 1 comments

Commit Message: //test/common/common:bit_array_test fails on big endian(s390x) as safeMemcpyUnsafeDst doesn't return expected values. Additional Description: Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional API Considerations:]

namrata-ibm avatar May 31 '24 05:05 namrata-ibm

Hi @namrata-ibm, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34441 was opened by namrata-ibm.

see: more, trace.

Assigning @KBaichoo as bit-array creator /assign @KBaichoo

adisuissa avatar Jun 04 '24 04:06 adisuissa

Hi @KBaichoo On big endian, when loadUnsignedWord/storeUnsignedWord are invoked and further bits are shifted/bitwise-ops are performed in bit_array.h-get/set functions here and here the wrong bits are processed. Using absl::little_endian::Load64/Store64 does a memcpy but additionally byte swaps on big endian resulting in correct values.

Also an update : It was pointed out in another PR that absl::little_endian are from internal file and should not be used. I will look for alternative and update this PR.

Thank you for looking into this.

namrata-ibm avatar Jun 05 '24 11:06 namrata-ibm

@KBaichoo Could you please have a look? Thank you.

namrata-ibm avatar Jun 12 '24 11:06 namrata-ibm

Thank you @KBaichoo @adisuissa

namrata-ibm avatar Jun 14 '24 06:06 namrata-ibm