Fix a bug in preprocessor "busy" logic (#1875)

* Fix a bug in preprocessor "busy" logic

This bug manifested in both incorrect preprocessor output for certain complicated cases, and also (more importantly) a use-after-free issue.

One of the "clever" design choices in the Slang preprocessor implementation is that the set of "busy" macros during expansion is implicitly defined by a linked list of those invocations that are actively being read from as part of the input stack. This logic works very well for checking whether a macro name is busy before triggering expansion, and for computing what macros should be considered busy during expansion of an object-like macro.

The problem case here was with function-like macros where the preprocessor was re-using the same list of busy macros that had been fetched when reading the macro name, but doing so *after* all the macro argument tokens had been read. Because additional tokens had been read from the input stream stack, there was no guarantee that the invocations that had been active before were still live.

The new logic computes the set of busy macros fresh before starting expansion of a function-like macro invocation. A test is included to ensure that the case that showed the use-after-free bug has been fixed.

In addition, the new logic is careful to compute the busy macros only based on where subsequent tokens will come from and not based on any macros that might have contributed to the argument tokens themselves. A test case has been added that relies on getting this detail correct.

* Update slang-preprocessor.cpp

Remove a test typo.

Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>

4 files changed, 53 insertions, 0 deletions

diff --git a/tests/preprocessor/bugs/pp-bug-1.slang b/tests/preprocessor/bugs/pp-bug-1.slang
new file mode 100644
index 000000000..b0f026a6b
--- /dev/null
+++ b/tests/preprocessor/bugs/pp-bug-1.slang
@@ -0,0 +1,20 @@
+// pp-bug-1.slang
+//DIAGNOSTIC_TEST:SIMPLE:-E
+
+// The bug in this case was related to a use-after-free
+// where the list of "busy" macros for a function-like
+// macro invocation was being set based on what was
+// busy when reading the macro name, which could include
+// streams that had been popped by the time arguments
+// had been read.
+
+#define NUM_CASES 2
+#define X1(M) M(0)
+#define X2(M) M(0) M(1)
+#define CONCAT2(a, b) a##b
+#define CONCAT(a, b) CONCAT2(a, b)
+#define FOREACH(M) CONCAT(X, NUM_CASES)(M)
+#define CASE(i) i
+// Should output: 0 1
+// Outputs: 0 CASE ( 1 )
+FOREACH(CASE)
\ No newline at end of file
diff --git a/tests/preprocessor/bugs/pp-bug-1.slang.expected b/tests/preprocessor/bugs/pp-bug-1.slang.expected
new file mode 100644
index 000000000..522fc1b40
--- /dev/null
+++ b/tests/preprocessor/bugs/pp-bug-1.slang.expected
@@ -0,0 +1,6 @@
+result code = 0
+standard error = {
+}
+standard output = {
+0 1 
+}
diff --git a/tests/preprocessor/macros/macro-parens-from-expansion.slang b/tests/preprocessor/macros/macro-parens-from-expansion.slang
new file mode 100644
index 000000000..3886c8db9
--- /dev/null
+++ b/tests/preprocessor/macros/macro-parens-from-expansion.slang
@@ -0,0 +1,21 @@
+// macro-parens-from-expansion.slang
+//DIAGNOSTIC_TEST:SIMPLE:-E
+
+// This test covers a case where the parentheses for
+// a function-like macro invocation argument list are
+// themselves produced by a macro invocation.
+//
+// In the below code, it is important that the macros
+// `LPAREN` and `RPAREN` are not considered busy for
+// the invocation of `INNER`, despite the fact that
+// both of those macros were expanded as part of
+// producing the arguments to the `INNER` invocation.
+
+#define LPAREN (
+#define RPAREN )
+#define INNER(X) LPAREN X RPAREN
+#define M(X) X
+#define OUTER(X) M( INNER LPAREN X RPAREN )
+
+// output: ( 3 )
+OUTER(3)
diff --git a/tests/preprocessor/macros/macro-parens-from-expansion.slang.expected b/tests/preprocessor/macros/macro-parens-from-expansion.slang.expected
new file mode 100644
index 000000000..96558709e
--- /dev/null
+++ b/tests/preprocessor/macros/macro-parens-from-expansion.slang.expected
@@ -0,0 +1,6 @@
+result code = 0
+standard error = {
+}
+standard output = {
+( 3 ) 
+}