From 797c3c4f5dc4fda735e55c6b5d6270a54cf6d263 Mon Sep 17 00:00:00 2001 From: Chris Lattner <sabre@nondot.org> Date: Mon, 10 Aug 2009 19:03:04 +0000 Subject: [PATCH] fix a couple of problems with section attributes: 1. Passing something that isn't a string used to cause: "argument to annotate attribute was not a string literal" make it say "section attribute" instead. 2. Fix the location of the above message to point to the bad argument instead of the section token. 3. Implement rdar://4341926, by diagnosing invalid section specifiers in the frontend rather than letting them slip all the way to the assembler (a QoI win). An example of #3 is that we used to produce something like this: /var/folders/n7/n7Yno9ihEm894640nJdSQU+++TI/-Tmp-//ccFPFGtT.s:2:Expected comma after segment-name /var/folders/n7/n7Yno9ihEm894640nJdSQU+++TI/-Tmp-//ccFPFGtT.s:2:Rest of line ignored. 1st junk character valued 46 (.). Daniel improved clang to use llvm_report_error, so now we got: $ clang t.c -c fatal error: error in backend: Global variable 'x' has an invalid section specifier 'sadf': mach-o section specifier requires a segment and section separated by a comma. with no loc info. Now we get: $ clang t.c -fsyntax-only t.c:4:30: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma int x __attribute__((section("sadf"))); ^ which is nice :) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@78586 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 6 +++-- include/clang/Basic/TargetInfo.h | 22 ++++++++++++++-- lib/Basic/Targets.cpp | 14 +++++++++- lib/Sema/SemaDeclAttr.cpp | 30 ++++++++++++++-------- test/Sema/attr-section.c | 10 ++++++++ tools/index-test/CMakeLists.txt | 1 + tools/index-test/Makefile | 2 +- tools/wpa/CMakeLists.txt | 1 + tools/wpa/Makefile | 2 +- 9 files changed, 71 insertions(+), 17 deletions(-) create mode 100644 test/Sema/attr-section.c diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 61e492b6f6d..969f23ea6ca 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -548,8 +548,10 @@ def err_implicit_pointer_address_space_cast : Error< "illegal implicit cast between two pointers with different address spaces">; def err_as_qualified_auto_decl : Error< "automatic variable qualified with an address space">; -def err_attribute_annotate_no_string : Error< - "argument to annotate attribute was not a string literal">; +def err_attribute_not_string : Error< + "argument to %0 attribute was not a string literal">; +def err_attribute_section_invalid_for_target : Error< + "argument to 'section' attribute is not valid for this target: %0">; def err_attribute_aligned_not_power_of_two : Error< "requested alignment is not a power of 2">; def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning< diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h index 8aa3378fec6..a34b8058c8f 100644 --- a/include/clang/Basic/TargetInfo.h +++ b/include/clang/Basic/TargetInfo.h @@ -21,14 +21,17 @@ #include <vector> #include <string> -namespace llvm { struct fltSemantics; } +namespace llvm { +struct fltSemantics; +class StringRef; +} namespace clang { class Diagnostic; +class SourceLocation; class SourceManager; class LangOptions; - namespace Builtin { struct Info; } /// TargetInfo - This class exposes information about the current target. @@ -324,6 +327,21 @@ public: virtual const char *getCFStringDataSection() const { return "__TEXT,__cstring,cstring_literals"; } + + + /// isValidSectionSpecifier - This is an optional hook that targets can + /// implement to perform semantic checking on attribute((section("foo"))) + /// specifiers. In this case, "foo" is passed in to be checked. If the + /// section specifier is invalid, the backend should return a non-empty string + /// that indicates the problem. + /// + /// This hook is a simple quality of implementation feature to catch errors + /// and give good diagnostics in cases when the assembler or code generator + /// would otherwise reject the section specifier. + /// + virtual std::string isValidSectionSpecifier(const llvm::StringRef &SR) const { + return ""; + } /// getDefaultLangOptions - Allow the target to specify default settings for /// various language options. These may be overridden by command line diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index 51f5c87803e..605ee8c8d87 100644 --- a/lib/Basic/Targets.cpp +++ b/lib/Basic/Targets.cpp @@ -19,6 +19,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/APFloat.h" #include "llvm/ADT/SmallString.h" +#include "llvm/MC/MCSectionMachO.h" using namespace clang; //===----------------------------------------------------------------------===// @@ -61,6 +62,7 @@ static void DefineStd(std::vector<char> &Buf, const char *MacroName, //===----------------------------------------------------------------------===// // Defines specific to certain operating systems. //===----------------------------------------------------------------------===// + namespace { template<typename TgtInfo> class OSTargetInfo : public TgtInfo { @@ -78,7 +80,7 @@ public: }; } -namespace { + /// getDarwinNumber - Parse the 'darwin number' out of the specific targe /// triple. For example, if we have darwin8.5 return 8,5,0. If any entry is /// not defined, return 0's. Return true if we have -darwin in the string or @@ -216,6 +218,7 @@ static void GetDarwinLanguageOptions(LangOptions &Opts, Opts.ObjCNonFragileABI = 1; } +namespace { template<typename Target> class DarwinTargetInfo : public OSTargetInfo<Target> { protected: @@ -245,8 +248,17 @@ public: virtual const char *getUnicodeStringSection() const { return "__TEXT,__ustring"; } + + virtual std::string isValidSectionSpecifier(const llvm::StringRef &SR) const { + // Let MCSectionMachO validate this. + llvm::StringRef Segment, Section; + unsigned TAA, StubSize; + return llvm::MCSectionMachO::ParseSectionSpecifier(SR, Segment, Section, + TAA, StubSize); + } }; + // DragonFlyBSD Target template<typename Target> class DragonFlyBSDTargetInfo : public OSTargetInfo<Target> { diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 9d1b0849eff..d5d3840569b 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -17,7 +17,7 @@ #include "clang/AST/Expr.h" #include "clang/Basic/TargetInfo.h" #include "clang/Parse/DeclSpec.h" -#include <llvm/ADT/StringExtras.h> +#include "llvm/ADT/StringExtras.h" using namespace clang; //===----------------------------------------------------------------------===// @@ -981,15 +981,25 @@ static void HandleSectionAttr(Decl *D, const AttributeList &Attr, Sema &S) { // Make sure that there is a string literal as the sections's single // argument. - StringLiteral *SE = - dyn_cast<StringLiteral>(static_cast<Expr *>(Attr.getArg(0))); + Expr *ArgExpr = static_cast<Expr *>(Attr.getArg(0)); + StringLiteral *SE = dyn_cast<StringLiteral>(ArgExpr); if (!SE) { - // FIXME - S.Diag(Attr.getLoc(), diag::err_attribute_annotate_no_string); + S.Diag(ArgExpr->getLocStart(), diag::err_attribute_not_string) << "section"; return; } - D->addAttr(::new (S.Context) SectionAttr(std::string(SE->getStrData(), - SE->getByteLength()))); + + std::string SectionStr(SE->getStrData(), SE->getByteLength()); + + // If the target wants to validate the section specifier, make it happen. + std::string Error = S.Context.Target.isValidSectionSpecifier(SectionStr); + if (Error.empty()) { + D->addAttr(::new (S.Context) SectionAttr(SectionStr)); + return; + } + + S.Diag(SE->getLocStart(), diag::err_attribute_section_invalid_for_target) + << Error; + } static void HandleStdCallAttr(Decl *d, const AttributeList &Attr, Sema &S) { @@ -1405,13 +1415,13 @@ static void HandleAnnotateAttr(Decl *d, const AttributeList &Attr, Sema &S) { S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 1; return; } - Expr *argExpr = static_cast<Expr *>(Attr.getArg(0)); - StringLiteral *SE = dyn_cast<StringLiteral>(argExpr); + Expr *ArgExpr = static_cast<Expr *>(Attr.getArg(0)); + StringLiteral *SE = dyn_cast<StringLiteral>(ArgExpr); // Make sure that there is a string literal as the annotation's single // argument. if (!SE) { - S.Diag(Attr.getLoc(), diag::err_attribute_annotate_no_string); + S.Diag(ArgExpr->getLocStart(), diag::err_attribute_not_string) <<"annotate"; return; } d->addAttr(::new (S.Context) AnnotateAttr(std::string(SE->getStrData(), diff --git a/test/Sema/attr-section.c b/test/Sema/attr-section.c new file mode 100644 index 00000000000..c61ed80cbf7 --- /dev/null +++ b/test/Sema/attr-section.c @@ -0,0 +1,10 @@ +// RUN: clang-cc -verify -fsyntax-only -triple x86_64-apple-darwin9 %s + +int x __attribute__((section( + 42))); // expected-error {{argument to section attribute was not a string literal}} + + +// rdar://4341926 +int y __attribute__((section( + "sadf"))); // expected-error {{mach-o section specifier requires a segment and section separated by a comma}} + diff --git a/tools/index-test/CMakeLists.txt b/tools/index-test/CMakeLists.txt index 880539bbe31..9c9656a1130 100644 --- a/tools/index-test/CMakeLists.txt +++ b/tools/index-test/CMakeLists.txt @@ -11,6 +11,7 @@ set( LLVM_USED_LIBS set( LLVM_LINK_COMPONENTS bitreader + mc ) add_clang_executable(index-test diff --git a/tools/index-test/Makefile b/tools/index-test/Makefile index cec0eee2861..0a43d2f47f9 100644 --- a/tools/index-test/Makefile +++ b/tools/index-test/Makefile @@ -17,7 +17,7 @@ TOOL_NO_EXPORTS = 1 include $(LEVEL)/Makefile.config -LINK_COMPONENTS := bitreader +LINK_COMPONENTS := bitreader mc USEDLIBS = clangIndex.a clangFrontend.a clangSema.a clangAST.a clangLex.a clangBasic.a include $(LLVM_SRC_ROOT)/Makefile.rules diff --git a/tools/wpa/CMakeLists.txt b/tools/wpa/CMakeLists.txt index 5ba3bc8a18b..5553474b4bc 100644 --- a/tools/wpa/CMakeLists.txt +++ b/tools/wpa/CMakeLists.txt @@ -11,6 +11,7 @@ set( LLVM_USED_LIBS ) set( LLVM_LINK_COMPONENTS + mc ) add_clang_executable(clang-wpa diff --git a/tools/wpa/Makefile b/tools/wpa/Makefile index c15be3f828d..ddde5b288bc 100644 --- a/tools/wpa/Makefile +++ b/tools/wpa/Makefile @@ -9,7 +9,7 @@ TOOL_NO_EXPORTS = 1 include $(LEVEL)/Makefile.config -LINK_COMPONENTS := bitreader +LINK_COMPONENTS := bitreader mc USEDLIBS = clangFrontend.a clangSema.a clangAST.a clangLex.a clangBasic.a clangAnalysis.a clangIndex.a include $(LLVM_SRC_ROOT)/Makefile.rules -- GitLab