On Tue, Jan 17, 2023 at 05:57:41PM +0100, Sebastien Marie wrote:
> 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
> > >
again, a new diff.
in C code, when using asm("") with .byte directive, the warning is sensible to
-Werror, and it broke the tree (on some archs, we are using such directive to
put instructions unknown from LLVM 13).
the main drawback of the diff is some context (like code from macro) could be
missing now:
$ cat -n test.c
1 #define TEST() asm(".byte 0xAA")
2
3 int
4 test(void)
5 {
6 TEST();
7 return 0;
8 }
$ cc -c test.c -Werror
<inline asm>:1:8: warning: directive value inside .text section: directive
'.byte', section '.text'
.byte 0xAA
^
$ echo $?
0
before, we had TEST() macro information...
$ cc -c test.c -Werror
test.c:6:2: error: directive value inside .text section: directive '.byte',
section '.text' [-Werror,-Winline-asm]
TEST();
^
test.c:1:20: note: expanded from macro 'TEST'
#define TEST() asm(".byte 0xAA")
^
<inline asm>:1:8: note: instantiated into assembly here
.byte 0xAA
^
1 error generated.
$ echo $?
1
I will look if I could do a bit better, but the following diff is immune to
-Werror.
--
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,18 @@ 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.")) {
+ const SMDiagnostic diag = SrcMgr.GetMessage(ExprLoc,
SourceMgr::DK_Warning,
+ "directive value inside .text section: "
+ "directive '" + Twine(IDVal) + "', section '" + Twine(sectionName) +
"'",
+ None, None);
+ unsigned CurBuf = SrcMgr.FindBufferContainingLoc(diag.getLoc());
+ SrcMgr.PrintIncludeStack(SrcMgr.getBufferInfo(CurBuf).IncludeLoc,
errs());
+ diag.print(nullptr, errs(), false);
+ }
// 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,21 @@ 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.")) {
+ const SMLoc ExprLoc = getLexer().getLoc();
+ const SMDiagnostic diag = SrcMgr.GetMessage(ExprLoc,
SourceMgr::DK_Warning,
+ "directive value inside .text section: "
+ "directive '" + Twine(IDVal) + "', section '" + Twine(sectionName) +
"'",
+ None, None);
+ unsigned CurBuf = SrcMgr.FindBufferContainingLoc(diag.getLoc());
+ SrcMgr.PrintIncludeStack(SrcMgr.getBufferInfo(CurBuf).IncludeLoc,
errs());
+ diag.print(nullptr, errs(), false);
+ }
+ }
if (StructInProgress.empty()) {
// Initialize data value.
if (emitIntegralValues(Size))
@@ -3825,6 +3840,21 @@ 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.")) {
+ const SMLoc ExprLoc = getLexer().getLoc();
+ const SMDiagnostic diag = SrcMgr.GetMessage(ExprLoc,
SourceMgr::DK_Warning,
+ "directive value inside .text section: "
+ "directive '" + Twine(Name) + "', section '" + Twine(sectionName) +
"'",
+ None, None);
+ unsigned CurBuf = SrcMgr.FindBufferContainingLoc(diag.getLoc());
+ SrcMgr.PrintIncludeStack(SrcMgr.getBufferInfo(CurBuf).IncludeLoc,
errs());
+ diag.print(nullptr, errs(), false);
+ }
+ }
// Initialize named data value.
MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
getStreamer().emitLabel(Sym);
