[all-commits] [llvm/llvm-project] 6175f0: Extend life of variables in `DiagComparison` in `E...

Tacet via All-commits all-commits at lists.llvm.org
Thu Jan 25 17:47:57 PST 2024


  Branch: refs/heads/users/AdvenamTacet/short-string-annotations-v4-testing
  Home:   https://github.com/llvm/llvm-project
  Commit: 6175f0bf3205e608d0bbdbd29c494563dfe00134
      https://github.com/llvm/llvm-project/commit/6175f0bf3205e608d0bbdbd29c494563dfe00134
  Author: Advenam Tacet <advenam.tacet at trailofbits.com>
  Date:   2024-01-26 (Fri, 26 Jan 2024)

  Changed paths:
    M clang/lib/AST/ExprConstant.cpp

  Log Message:
  -----------
  Extend life of variables in `DiagComparison` in `ExprConstant`

This commit makes two variables static extending their life span.
This patch is designed to address the issue of buildbots failing when AddressSanitizer's (ASan) short string annotations are enabled.
It's esentially same as:
- https://github.com/llvm/llvm-project/pull/79489
however, it's less likely to solve the real problem as those strings change (aren't `const`).
I suspect that there may be use after end of life bug (in StringRef), but it requires confirmation.
In that case, one alternative solution, which unfortunately results in memory leaks, is to always allocate new strings instead of overwriting existing (static) ones. This approach would prevent potential data corruption, but I don't suggest it in this PR.

This patch makes `Clang :: SemaCXX/builtins.cpp` test pass with short string annotations (ASan).
With https://github.com/llvm/llvm-project/pull/79489 it fixes known problems with buildbots, while running with short string annotations.
However, the potential issue still requires more investigation therefore FIXME comment is added in that patch.

Short string annotations PR (reverted):
- https://github.com/llvm/llvm-project/pull/79049

Buildbots (failure) output:
- https://lab.llvm.org/buildbot/#/builders/5/builds/40364/steps/9/logs/stdio

While buildbots should not fail with proposed changes, we still should investigate why buildbots were failing with ASan short string annotations turned on.
StringRef objects (made from those strings) can potentially change their contents unexpectedly or even (potentially) use of freed memory may happen.
That interpretation is only my educated guess, I still didn't understand exactly why those buildbots are failing.


  Commit: 1a3e00bfe2fa8e3ba6d1bedd26ab5d768a19e68c
      https://github.com/llvm/llvm-project/commit/1a3e00bfe2fa8e3ba6d1bedd26ab5d768a19e68c
  Author: Advenam Tacet <advenam.tacet at trailofbits.com>
  Date:   2024-01-26 (Fri, 26 Jan 2024)

  Changed paths:
    M clang/lib/AST/ExprConstant.cpp

  Log Message:
  -----------
  additional comment


  Commit: 19fee27d488b9722e1ab55cbc160faa375321c04
      https://github.com/llvm/llvm-project/commit/19fee27d488b9722e1ab55cbc160faa375321c04
  Author: Tacet <advenam.tacet at trailofbits.com>
  Date:   2024-01-26 (Fri, 26 Jan 2024)

  Changed paths:
    M libcxx/include/string
    A libcxx/test/libcxx/containers/strings/basic.string/asan_deque_integration.pass.cpp
    A libcxx/test/libcxx/containers/strings/basic.string/asan_short.pass.cpp
    A libcxx/test/libcxx/containers/strings/basic.string/asan_vector_integration.pass.cpp
    M libcxx/test/support/asan_testing.h

  Log Message:
  -----------
  Turn on ASan annotations for short strings (#79049)

Originally merged here: https://github.com/llvm/llvm-project/pull/75882
Reverted here: https://github.com/llvm/llvm-project/pull/78627

Reverted due to failing buildbots. The problem was not caused by the
annotations code, but by code in the `UniqueFunctionBase` class and in
the `JSON.h` file. That code caused the program to write to memory that
was already being used by string objects, which resulted in an ASan
error.

Fixes are implemented in:
- https://github.com/llvm/llvm-project/pull/79065
- https://github.com/llvm/llvm-project/pull/79066

Problematic code from `UniqueFunctionBase` for example:
```cpp
    // In debug builds, we also scribble across the rest of the storage.
    memset(RHS.getInlineStorage(), 0xAD, InlineStorageSize);
```


Compare: https://github.com/llvm/llvm-project/compare/6175f0bf3205%5E...19fee27d488b


More information about the All-commits mailing list