Skip to content

[RISCV][compiler-rt] Small fixes for __riscv_feature_bits #100158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

BeMg
Copy link
Contributor

@BeMg BeMg commented Jul 23, 2024

Changes included:

  • Adding CONSTRUCTOR_ATTRIBUTE so that the static data is setup early on in process lifetime. This is required by gcc docs for __builtin_cpu_supports which we hope to implement in terms of this.
  • Move the length initialization outside of the #if defined(linux) block so that the length field always reflects the size of the structures even if non of the feature bits are non-zero.
  • Change the __riscv_vendor_feature_bits.length field to match the length of the actual structure.

Note: Copy from #99958

preames added 4 commits July 22, 2024 14:56
Changes included:
* Adding CONSTRUCTOR_ATTRIBUTE so that the static data is setup early on
  in process lifetime.  This is required by gcc docs for
  __builtin_cpu_supports which we hope to implement in terms of this.
* Move the length initialization outside of the #if defined(__linux__)
  block so that the length field always reflects the size of the structures
  even if non of the feature bits are non-zero.
* Change the __riscv_vendor_feature_bits.length field to match the length
  of the actual structure.

Note that this change has not been built or tested.  I could not figure
out how to get a working cross build for compiler-rt setup.
@BeMg
Copy link
Contributor Author

BeMg commented Jul 23, 2024

After moving to cpu_model directory, compiler-rt could build successfully.

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM

JFYI, since this seems to be textually the same as the final iteration on my review, I'm going to treat you posting this as an approval on that one. I don't care which one lands. I'm about to run out for an errand, and if this hasn't landed by the time I take another look this afternoon I'll land one or the other.

@BeMg BeMg merged commit 73ac953 into llvm:main Jul 23, 2024
12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 24, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux running on sanitizer-buildbot2 while building compiler-rt at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/2033

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[372/377] Generating MSAN_INST_GTEST.gtest-all.cc.x86_64.o
[373/377] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[374/377] Generating Msan-x86_64-with-call-Test
[375/377] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[376/377] Generating Msan-x86_64-Test
[376/377] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 10055 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60..
FAIL: SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp (7038 of 10055)
******************** TEST 'SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=leak  -m32 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m32 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
RUN: at line 5: env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1      /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp 2>&1 | FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
+ env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp:68:24: error: CHECK_MAY_RETURN_1: expected string not found in input
// CHECK_MAY_RETURN_1: allocating 512 times
                       ^
<stdin>:52:44: note: scanning from here
Some of the malloc calls returned non-null: 256
                                           ^
<stdin>:53:14: note: possible intended match here
==1752561==LeakSanitizer: soft rss limit unexhausted (220Mb vs 3Mb)
             ^

Input file: <stdin>
Check file: /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           47:  [256] 
           48:  [320] 
           49:  [384] 
           50:  [448] 
           51: Some of the malloc calls returned null: 256 
           52: Some of the malloc calls returned non-null: 256 
check:68'0                                                X~~~~ error: no match found
           53: ==1752561==LeakSanitizer: soft rss limit unexhausted (220Mb vs 3Mb) 
Step 14 (test compiler-rt default) failure: test compiler-rt default (failure)
...
[372/377] Generating MSAN_INST_GTEST.gtest-all.cc.x86_64.o
[373/377] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[374/377] Generating Msan-x86_64-with-call-Test
[375/377] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[376/377] Generating Msan-x86_64-Test
[376/377] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 10055 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60..
FAIL: SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp (7038 of 10055)
******************** TEST 'SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=leak  -m32 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m32 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
RUN: at line 5: env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1      /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp 2>&1 | FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
+ env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp:68:24: error: CHECK_MAY_RETURN_1: expected string not found in input
// CHECK_MAY_RETURN_1: allocating 512 times
                       ^
<stdin>:52:44: note: scanning from here
Some of the malloc calls returned non-null: 256
                                           ^
<stdin>:53:14: note: possible intended match here
==1752561==LeakSanitizer: soft rss limit unexhausted (220Mb vs 3Mb)
             ^

Input file: <stdin>
Check file: /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           47:  [256] 
           48:  [320] 
           49:  [384] 
           50:  [448] 
           51: Some of the malloc calls returned null: 256 
           52: Some of the malloc calls returned non-null: 256 
check:68'0                                                X~~~~ error: no match found
           53: ==1752561==LeakSanitizer: soft rss limit unexhausted (220Mb vs 3Mb) 

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
Changes included:

- Adding CONSTRUCTOR_ATTRIBUTE so that the static data is setup early on
in process lifetime. This is required by gcc docs for
__builtin_cpu_supports which we hope to implement in terms of this.
- Move the length initialization outside of the #if defined(linux) block
so that the length field always reflects the size of the structures even
if non of the feature bits are non-zero.
- Change the __riscv_vendor_feature_bits.length field to match the
length of the actual structure.

Note: Copy from #99958

---------

Co-authored-by: Philip Reames <[email protected]>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250767
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants