Page MenuHomePhabricator

[compiler-rt] Silence warnings about large stack frames in ReadBinaryName
AbandonedPublic

Authored by mstorsjo on Feb 26 2021, 2:34 PM.

Details

Reviewers
akhuang
rnk
Summary

The disabling of the warning also needs to cover ReadLongProcessName, which ends up inlining the function and producing a separate warning about stack size.

Other cases of the same were silenced similarly in D91853.

Diff Detail

Event Timeline

mstorsjo created this revision.Feb 26 2021, 2:34 PM
mstorsjo requested review of this revision.Feb 26 2021, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2021, 2:34 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
rnk added a comment.Mar 1 2021, 11:02 AM

I dug back to 2014 to find out why we have this flag: rG167c15a98feb898c13eca325678b73d0c59dffa2 Unfortunately, it seems there is no rationale. I'm guessing it's carried over from TSan, which has this flag. TSan uses sanitizer_common, so if tsan cares about frame size, then sanitizer_common does. The TSan warning flag comes from rG06379b35373e0ca9561c130284104fbdf0295f79, but again, no rationale or link to a review thread. =/

Instead of disabling the warning and adding ifdefs, maybe it's better to just fix the warning with InternalMmapVector<wchar_t>. The vast majority of uses of kMaxPathLength are for local InternalScopedString variables.

In D97579#2594982, @rnk wrote:

I dug back to 2014 to find out why we have this flag: rG167c15a98feb898c13eca325678b73d0c59dffa2 Unfortunately, it seems there is no rationale. I'm guessing it's carried over from TSan, which has this flag. TSan uses sanitizer_common, so if tsan cares about frame size, then sanitizer_common does. The TSan warning flag comes from rG06379b35373e0ca9561c130284104fbdf0295f79, but again, no rationale or link to a review thread. =/

Instead of disabling the warning and adding ifdefs, maybe it's better to just fix the warning with InternalMmapVector<wchar_t>. The vast majority of uses of kMaxPathLength are for local InternalScopedString variables.

Thanks for digging in! I've added a couple similar ones before - I can undo those and use InternalMmapVector<> for those too - but I don't (yet) have a setup where I can easily run the sanitizer tests - do you happen to have that? I'll post an alternative patch with that in any case.

mstorsjo abandoned this revision.Apr 19 2021, 1:18 PM

Superseded by D97726.