Re: help wanted for a specific clang diff

[Sebastien Marie]

On Tue, Jan 17, 2023 at 02:14:20PM +0100, Sebastien Marie wrote:
> 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.
> 

New diff with warning message simpler to grep.

  test.S:29:8: warning: directive value inside .text section: directive 
'.byte', section '.text'

$ grep -F "directive value inside .text section:"

-- 
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 inside .text section: "
+        "directive '" + Twine(IDVal) + "', section '" + Twine(sectionName) + 
"'");
     // 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 inside .text section: "
+        "directive '" + Twine(IDVal) + "', section '" + Twine(sectionName) + 
"'");
+  }
   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 value inside .text section: "
+          "named directive '" + Twine(Name) + "', section '" + 
Twine(sectionName) + "'");
+    }
     // Initialize named data value.
     MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
     getStreamer().emitLabel(Sym);