Skip to content

[AsmParser] Correctly handle .ifeqs nested in other conditional directives #132713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 24, 2025

Conversation

ostannard
Copy link
Collaborator

The parser function used for the .ifeqs and .ifnes directives was missing the check for whether we are currently in an ignored block of an outer conditional directive, causing the block to be evaluated when it should not, for example:

.if 0
.ifeqs "a", "a"
// Should not be evaluated, but is
nop
.endif
.endif

…tives

The parser function used for the .ifeqs and .ifnes directives was
missing the check for whether we are currently in an ignored block of an
outer conditional directive, causing the block to be evaluated when it
should not, for example:

.if 0
  .ifeqs "a", "a"
    // Should not be evaluated, but is
    nop
  .endif
.endif
@ostannard ostannard requested review from compnerd and smithp35 March 24, 2025 11:01
@llvmbot llvmbot added the mc Machine (object) code label Mar 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 24, 2025

@llvm/pr-subscribers-mc

Author: Oliver Stannard (ostannard)

Changes

The parser function used for the .ifeqs and .ifnes directives was missing the check for whether we are currently in an ignored block of an outer conditional directive, causing the block to be evaluated when it should not, for example:

.if 0
.ifeqs "a", "a"
// Should not be evaluated, but is
nop
.endif
.endif


Full diff: https://github.com/llvm/llvm-project/pull/132713.diff

3 Files Affected:

  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+30-25)
  • (modified) llvm/test/MC/AsmParser/ifeqs.s (+11)
  • (modified) llvm/test/MC/AsmParser/ifnes.s (+10)
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index c11c01e52fc2e..b0f0c5e5db2ce 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -5224,37 +5224,42 @@ bool AsmParser::parseDirectiveIfc(SMLoc DirectiveLoc, bool ExpectEqual) {
 /// parseDirectiveIfeqs
 ///   ::= .ifeqs string1, string2
 bool AsmParser::parseDirectiveIfeqs(SMLoc DirectiveLoc, bool ExpectEqual) {
-  if (Lexer.isNot(AsmToken::String)) {
-    if (ExpectEqual)
-      return TokError("expected string parameter for '.ifeqs' directive");
-    return TokError("expected string parameter for '.ifnes' directive");
-  }
+  TheCondStack.push_back(TheCondState);
+  TheCondState.TheCond = AsmCond::IfCond;
 
-  StringRef String1 = getTok().getStringContents();
-  Lex();
+  if (TheCondState.Ignore) {
+    eatToEndOfStatement();
+  } else {
+    if (Lexer.isNot(AsmToken::String)) {
+      if (ExpectEqual)
+        return TokError("expected string parameter for '.ifeqs' directive");
+      return TokError("expected string parameter for '.ifnes' directive");
+    }
 
-  if (Lexer.isNot(AsmToken::Comma)) {
-    if (ExpectEqual)
-      return TokError(
-          "expected comma after first string for '.ifeqs' directive");
-    return TokError("expected comma after first string for '.ifnes' directive");
-  }
+    StringRef String1 = getTok().getStringContents();
+    Lex();
 
-  Lex();
+    if (Lexer.isNot(AsmToken::Comma)) {
+      if (ExpectEqual)
+        return TokError(
+            "expected comma after first string for '.ifeqs' directive");
+      return TokError("expected comma after first string for '.ifnes' directive");
+    }
 
-  if (Lexer.isNot(AsmToken::String)) {
-    if (ExpectEqual)
-      return TokError("expected string parameter for '.ifeqs' directive");
-    return TokError("expected string parameter for '.ifnes' directive");
-  }
+    Lex();
 
-  StringRef String2 = getTok().getStringContents();
-  Lex();
+    if (Lexer.isNot(AsmToken::String)) {
+      if (ExpectEqual)
+        return TokError("expected string parameter for '.ifeqs' directive");
+      return TokError("expected string parameter for '.ifnes' directive");
+    }
 
-  TheCondStack.push_back(TheCondState);
-  TheCondState.TheCond = AsmCond::IfCond;
-  TheCondState.CondMet = ExpectEqual == (String1 == String2);
-  TheCondState.Ignore = !TheCondState.CondMet;
+    StringRef String2 = getTok().getStringContents();
+    Lex();
+
+    TheCondState.CondMet = ExpectEqual == (String1 == String2);
+    TheCondState.Ignore = !TheCondState.CondMet;
+  }
 
   return false;
 }
diff --git a/llvm/test/MC/AsmParser/ifeqs.s b/llvm/test/MC/AsmParser/ifeqs.s
index 05a26a237fcd3..fd5f1a84980a2 100644
--- a/llvm/test/MC/AsmParser/ifeqs.s
+++ b/llvm/test/MC/AsmParser/ifeqs.s
@@ -18,3 +18,14 @@
 // CHECK-NOT: .byte 0
 // CHECK: .byte 1
 
+
+.if 0
+  .ifeqs "alpha", "alpha"
+    .byte 2
+  .else
+    .byte 3
+  .endif
+.endif
+
+// CHECK-NOT: .byte 2
+// CHECK-NOT: .byte 3
diff --git a/llvm/test/MC/AsmParser/ifnes.s b/llvm/test/MC/AsmParser/ifnes.s
index 7a3cbe0264e1b..bd7dc3b637b21 100644
--- a/llvm/test/MC/AsmParser/ifnes.s
+++ b/llvm/test/MC/AsmParser/ifnes.s
@@ -20,3 +20,13 @@
 # CHECK: .byte 1
 .ifnes "equal", "equal" ; .byte 0 ; .else ; .byte 1 ; .endif
 
+.if 0
+  .ifnes "alpha", "alpha"
+    .byte 2
+  .else
+    .byte 3
+  .endif
+.endif
+
+// CHECK-NOT: .byte 2
+// CHECK-NOT: .byte 3

Copy link

github-actions bot commented Mar 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Every other parseDirectiveIf follows the proposed change.

@ostannard ostannard merged commit 7ada6f1 into llvm:main Mar 24, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants