You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fory.apache.org by "BaldDemian (via GitHub)" <gi...@apache.org> on 2026/03/08 09:52:55 UTC
[PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
BaldDemian opened a new pull request, #3458:
URL: https://github.com/apache/fory/pull/3458
<!--
**Thanks for contributing to Apache Fory™.**
**If this is your first time opening a PR on fory, you can refer to [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fory™** community has requirements on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).
- Apache Fory™ has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here.
-->
## Why?
Close https://github.com/apache/fory/issues/1203
## What does this PR do?
- Add ASan and UBSan integration in `.bazelrc`.
- I noticed that directly running `bazel test $(bazel query //cpp/...)` triggers compilation errors because the `--config=x86_64` flag is not provided. In the new CI Python script `run_ci.py`, the `cpp.py` task invoked by it indeed contains a check for this flag:
https://github.com/apache/fory/blob/af6e8b25e8c2d188cba2d99ccb276c47ec6c9c31/ci/tasks/cpp.py#L40-L41
The `run_ci.sh` script does not contain this check. Although this script appears to no longer be used, I still added the corresponding flag check there.
Similarly, I also added documentation about this flag in `AGENTS.md`.
- In `ci.yml`, I added a new job to enable tests with sanitizers in CI. Currently, this job only runs on ubuntu-latest.
## Related issues
https://github.com/apache/fory/issues/1203
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here.
Delete section if not applicable.
-->
I believe this feature only introduces extra burden during testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "BaldDemian (via GitHub)" <gi...@apache.org>.
BaldDemian closed pull request #3458: feat(c++): add sanitizer configuration in bazel build and CI
URL: https://github.com/apache/fory/pull/3458
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "BaldDemian (via GitHub)" <gi...@apache.org>.
BaldDemian commented on PR #3458:
URL: https://github.com/apache/fory/pull/3458#issuecomment-4018780150
[This sole failed CI job](https://github.com/apache/fory/actions/runs/22818663268/job/66187819482?pr=3458) seems to be a network issue 🤔. It passed in my local fork, so simply rerunning it may solve the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "BaldDemian (via GitHub)" <gi...@apache.org>.
BaldDemian commented on PR #3458:
URL: https://github.com/apache/fory/pull/3458#issuecomment-4020881256
After some investigation, I can confirm that the misaligned address access UB mentioned above is caused by the way `buffer.h` creates types that require alignment using `reinterpret_cast` directly, e.g.
https://github.com/apache/fory/blob/ed586af222327bb2c3e7220832c991f211920005/cpp/fory/util/buffer.h#L885-L892
In this method, `int16` requires 2 byte alignment, but `reinterpret_cast` cannot guarantee or check this requirement.
I am preparing a PR to fix this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "BaldDemian (via GitHub)" <gi...@apache.org>.
BaldDemian commented on PR #3458:
URL: https://github.com/apache/fory/pull/3458#issuecomment-4018764900
Results
Local testing:
- ASan:
Run `bazel test --cache_test_results=no --config=x86_64 --config=asan $(bazel query //...)`
All tests passed, and no memory access errors were detected. Logs are available in this file: [asan_report.txt](https://github.com/user-attachments/files/25822903/asan_report.txt)
- UBSan:
Run `bazel test --cache_test_results=no --config=x86_64 --config=ubsan $(bazel query //...)`
Normally, when integrating UBSan, it is necessary to add the following line in `.bazelrc`: `build:ubsan --copt=-fsanitize=undefined`.
However, with gcc, running the above `bazel test` command leads to a compilation failure.
See this file for details: [ubsan_compiling_error.txt](https://github.com/user-attachments/files/25822911/ubsan_compiling_error.txt).
This compilation failure appears to be caused by a known and unresolved bug in gcc. See this discussion for reference: https://stackoverflow.com/questions/79775145/adding-fsanitize-undefined-in-gcc-makes-reflect-cpp-header-not-compile-anymore
`undefined` is an aggregate sanitizer flag, and the list of its sub-options can be found here: https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fsanitize_003dundefined
After some investigation, it seems that enabling the sub-options `null`, `nonnull-attribute`, and `returns-nonnull-attribute` will all trigger the compilation failure. The current workaround is to manually expand the sub-options included in `undefined` and exclude the three options above. With this configuration, the compilation error no longer occurs.
Test results: several **misaligned address runtime error** was indeed detected.
However, due to the default behavior of UBSan(continuing execution even when errors are reported), the final result still shows that all tests passed. From the log file
[ubsan_report.txt](https://github.com/user-attachments/files/25822916/ubsan_report.txt)
(search for `runtime error`), the error appears to originate mainly from `buffer.h`. I will further investigate the code to check whether this is indeed a bug.
CI testing:
Newly added jobs and the existing ones all passed in my local fork: https://github.com/BaldDemian/fory/commit/95334b2bcd7ef8361b19398bd74869438a7dc7e8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "ayush00git (via GitHub)" <gi...@apache.org>.
ayush00git commented on PR #3458:
URL: https://github.com/apache/fory/pull/3458#issuecomment-4019192777
> [This sole failed CI job](https://github.com/apache/fory/actions/runs/22818663268/job/66187819482?pr=3458) seems to be a network issue 🤔. It passed in my local fork, so simply rerunning it may solve the problem.
pushing an empty commit would re-run the ci, try `git commit --allow-empty -m "ci"` and then push it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] feat(c++): add sanitizer configuration in bazel build and CI [fory]
Posted by "BaldDemian (via GitHub)" <gi...@apache.org>.
BaldDemian commented on PR #3458:
URL: https://github.com/apache/fory/pull/3458#issuecomment-4097293481
I manually closed this PR since [an upstream PR](https://github.com/apache/fory/pull/3479) has been merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org