Re: help wanted for a specific clang diff

[Sebastien Marie]

On Mon, Jan 16, 2023 at 12:00:30PM -0700, Theo de Raadt wrote:
> For this xonly work, we are having to one-by-one find .S files that
> are putting data tables into the .text segment
> 
> I am hoping to find someone who can do c++ well enough, and maybe
> has some familiarity with the clang code, to add a warning message
> for this
> 
> if a .long, .quad, .byte are placed into a .text section, issue
> a warning, then we'll be able to identify all these in a ports build
> and decide which need manual fixing, and move the objects into .rodata
> and apply __PIC__ handling as neccessary
> 
> Yes, there are cases where people use .long to inject an instruction
> they don't believe the assembler has.  We can ignore those by inspect.
> 
> Can anyone help?  It doesn't need to be fancy, it just needs to get
> us moving faster.
> 
> Thanks
> 

The following diff seems to work. It adds a warning inside assembler parsers.

Please note that it is a warning at assembler level, so -Werror doesn't 
influence it, whereas -Wa,--fatal-warnings will.


The check is done for "directive value" and "directive named value". It doesn't 
cover ascii directives (like .asci{i,z}, .string) neither real directives 
(.real{4,8,10}).


I checked it on somehow simple code:

$ clang -c test.S 
test.S:1:10: warning: directive value '.long' inside '.text' section
a: .long 0x90909090
         ^
test.S:2:10: warning: directive value '.word' inside '.text' section
b: .word 0x9090
         ^
test.S:3:10: warning: directive value '.quad' inside '.text' section
c: .quad 0x90
         ^
test.S:4:10: warning: directive value '.byte' inside '.text' section
d: .byte 0x90
         ^


-- 
Sebastien Marie


diff /home/semarie/repos/openbsd/src
commit - b18ed08defc1c5e5b4be701ce5ef913bda8ae66a
path + /home/semarie/repos/openbsd/src
blob - 047ed1660a837e77561b1f2839ec9601f9582a7e
file + gnu/llvm/llvm/lib/MC/MCParser/AsmParser.cpp
--- gnu/llvm/llvm/lib/MC/MCParser/AsmParser.cpp
+++ gnu/llvm/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -3168,6 +3168,12 @@ bool AsmParser::parseDirectiveValue(StringRef IDVal, u
     SMLoc ExprLoc = getLexer().getLoc();
     if (checkForValidSection() || parseExpression(Value))
       return true;
+    // Check for directive inside .text
+    const MCSection *Section = getStreamer().getCurrentSectionOnly();
+    const StringRef sectionName = Section->getName();
+    if (sectionName.equals(".text") || sectionName.startswith(".text."))
+      Warning(ExprLoc, "directive value '" + Twine(IDVal) + "' inside '"
+        + Twine(sectionName) + "' section");
     // Special case constant expressions to match code generator.
     if (const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(Value)) {
       assert(Size <= 8 && "Invalid size");
blob - 7b4d6e529cc2c3c4efce05f62f41620658d7a8e0
file + gnu/llvm/llvm/lib/MC/MCParser/MasmParser.cpp
--- gnu/llvm/llvm/lib/MC/MCParser/MasmParser.cpp
+++ gnu/llvm/llvm/lib/MC/MCParser/MasmParser.cpp
@@ -3809,6 +3809,14 @@ bool MasmParser::parseDirectiveValue(StringRef IDVal, 
 /// parseDirectiveValue
 ///  ::= (byte | word | ... ) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveValue(StringRef IDVal, unsigned Size) {
+  // Check for directive inside .text
+  const MCSection *Section = getStreamer().getCurrentSectionOnly();
+  if (Section != NULL) {
+    const StringRef sectionName = Section->getName();
+    if (sectionName.equals(".text") || sectionName.startswith(".text."))
+      Warning(getLexer().getLoc(), "directive value '" + Twine(IDVal)
+        + "' inside '" + Twine(sectionName) + "' section");
+  }
   if (StructInProgress.empty()) {
     // Initialize data value.
     if (emitIntegralValues(Size))
@@ -3825,6 +3833,14 @@ bool MasmParser::parseDirectiveNamedValue(StringRef Ty
 bool MasmParser::parseDirectiveNamedValue(StringRef TypeName, unsigned Size,
                                           StringRef Name, SMLoc NameLoc) {
   if (StructInProgress.empty()) {
+    // Check for directive inside .text
+    const MCSection *Section = getStreamer().getCurrentSectionOnly();
+    if (Section != NULL) {
+      const StringRef sectionName = Section->getName();
+      if (sectionName.equals(".text") || sectionName.startswith(".text."))
+        Warning(getLexer().getLoc(), "directive named value '" + Twine(Name)
+          + "' inside '" + Twine(sectionName) + "' section");
+    }
     // Initialize named data value.
     MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
     getStreamer().emitLabel(Sym);