Page MenuHomePhabricator

[Docs][Clang][Attr] mark no_stack_protector+no_sanitize GCC compatible
AbandonedPublic

Authored by nickdesaulniers on Jun 17 2021, 2:51 PM.

Details

Summary

GCC has supported the function attributes
attribute((no_stack_protector)) since GCC 11 and
attribute((no_sanitize(""))) since GCC 8.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Diff Detail

Unit TestsFailed

TimeTest
30 msx64 debian > Clang.AST::ast-print-no-sanitize.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/13.0.0/include -nostdsysteminc -std=c++11 -ast-print /var/lib/buildkite-agent/builds/llvm-project/clang/test/AST/ast-print-no-sanitize.cpp -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/AST/ast-print-no-sanitize.cpp
30 msx64 debian > Clang.SemaCXX::attr-no-sanitize.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/13.0.0/include -nostdsysteminc -std=c++11 -fsyntax-only -verify /var/lib/buildkite-agent/builds/llvm-project/clang/test/SemaCXX/attr-no-sanitize.cpp

Event Timeline

nickdesaulniers requested review of this revision.Jun 17 2021, 2:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2021, 2:51 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
melver accepted this revision.Jun 18 2021, 1:27 AM

There might be subtle inconsistencies between values accepted by our no_sanitize and GCC's no_sanitize, because of internal naming differences, but I couldn't say what those are right now (I think no_sanitize("coverage") only works with Clang, and GCC wants no_sanitize_coverage which we don't have due to not allowing new no_sanitize_*).

This revision is now accepted and ready to land.Jun 18 2021, 1:27 AM
aaron.ballman requested changes to this revision.Jun 18 2021, 3:47 AM

This change breaks code by removing the [[clang::no_sanitize]] spelling (and the fact that zero tests broke show we're missing test coverage here). We don't currently have a spelling to represent an attribute known to both Clang and GCC, nor do we currently have any attributes that use both the Clang and GCC spellings (which both include a GNU spelling that would be duplicated). I think you should keep the Clang spelling, add a GCC spelling, make sure that ClangAttrEmitter.cpp does not generate two GNU spellings for it, and add some basic test coverage for the change.

This revision now requires changes to proceed.Jun 18 2021, 3:47 AM
nickdesaulniers abandoned this revision.Jun 18 2021, 11:22 AM

ah, ok, then if this isn't a quick cleanup, I don't really care about this.

ah, ok, then if this isn't a quick cleanup, I don't really care about this.

I took a quick look at ClangAttrEmitter.cpp, and it looks like the only change that is needed is within the GetFlattenedSpelling() function. So not quite quick, but shouldn't be too bad (in case someone wants to pick this back up).