This is an archive of the discontinued LLVM Phabricator instance.

[Sanitizer] Fixup for ca50840b5bc0
ClosedPublic

Authored by yln on Sep 21 2022, 2:40 PM.

Diff Detail

Event Timeline

yln created this revision.Sep 21 2022, 2:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 2:40 PM
Herald added a subscriber: Enna1. · View Herald Transcript
yln requested review of this revision.Sep 21 2022, 2:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 2:41 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
yln added a comment.Sep 22 2022, 10:59 AM

@protze.joachim does this fix the error you see?

yln added a reviewer: thakis.Sep 23 2022, 12:22 PM

@protze.joachim does this fix the error you see?

Hi, Julian,

In total there are two errors in this file. Apart from the invocation of removed function "MaybeReexec", this file also misses a header file "sanitizer_common/sanitizer_interface_internal.h".
I have attached the compiler's error message in the following section for your reference.

/llvm-project/compiler-rt/lib/tsan/rtl-old/tsan_rtl.cpp:212:12: warning: variable 'i' set but not used [-Wunused-but-set-variable]

for (int i = 0;
         ^

/llvm-project/compiler-rt/lib/tsan/rtl-old/tsan_rtl.cpp:419:3: error: use of undeclared identifier 'MaybeReexec'

MaybeReexec();
^

/llvm-project/compiler-rt/lib/tsan/rtl-old/tsan_rtl.cpp:437:3: error: use of undeclared identifier '__sanitizer_set_report_path'

__sanitizer_set_report_path(common_flags()->log_path);
yln updated this revision to Diff 462603.Sep 23 2022, 4:04 PM

Add include

yln added a comment.Sep 23 2022, 4:08 PM

H @lechenyu, thanks for helping out! :)

this file also misses a header file "sanitizer_common/sanitizer_interface_internal.h".

This probably means that rtl-old is unused / bit rotting in more than one way. AFAIK, I didn't add this dependency in D129157.

I still added the #include, but it probably means we should remove rtl-old sooner than later.

@lechenyu: do you mind signing off on this?

@lechenyu: do you mind signing off on this?

I am sorry I forget to send you the confirmation. I have tested this patch last week and it did fix the issue.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 6 2022, 12:29 PM
This revision was automatically updated to reflect the committed changes.