Skip to content

[clang][Interp] Fix function pointer callexpr eval order #101821

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 1 commit into from
Aug 3, 2024

Conversation

tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Aug 3, 2024

We need to evaluate the callee before the arguments.

We need to evaluate the callee before the arguments.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 3, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We need to evaluate the callee before the arguments.


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

2 Files Affected:

  • (modified) clang/lib/AST/Interp/Compiler.cpp (+13-9)
  • (modified) clang/test/AST/Interp/eval-order.cpp (+2-3)
diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index e1fa0eb1eacb3..ada22b569b2b0 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -4003,6 +4003,13 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
     } else if (!this->visit(MC->getImplicitObjectArgument())) {
       return false;
     }
+  } else if (!FuncDecl) {
+    const Expr *Callee = E->getCallee();
+    CalleeOffset = this->allocateLocalPrimitive(Callee, PT_FnPtr, true, false);
+    if (!this->visit(Callee))
+      return false;
+    if (!this->emitSetLocal(PT_FnPtr, *CalleeOffset, E))
+      return false;
   }
 
   llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args);
@@ -4071,22 +4078,19 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
     for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I)
       ArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
 
-    // Get the callee, either from a member pointer saved in CalleeOffset,
-    // or by just visiting the Callee expr.
-    if (CalleeOffset) {
+    // Get the callee, either from a member pointer or function pointer saved in
+    // CalleeOffset.
+    if (isa<CXXMemberCallExpr>(E) && CalleeOffset) {
       if (!this->emitGetLocal(PT_MemberPtr, *CalleeOffset, E))
         return false;
       if (!this->emitGetMemberPtrDecl(E))
         return false;
-      if (!this->emitCallPtr(ArgSize, E, E))
-        return false;
     } else {
-      if (!this->visit(E->getCallee()))
-        return false;
-
-      if (!this->emitCallPtr(ArgSize, E, E))
+      if (!this->emitGetLocal(PT_FnPtr, *CalleeOffset, E))
         return false;
     }
+    if (!this->emitCallPtr(ArgSize, E, E))
+      return false;
   }
 
   // Cleanup for discarded return values.
diff --git a/clang/test/AST/Interp/eval-order.cpp b/clang/test/AST/Interp/eval-order.cpp
index 77f50831f4f47..d9cfd0b4642fa 100644
--- a/clang/test/AST/Interp/eval-order.cpp
+++ b/clang/test/AST/Interp/eval-order.cpp
@@ -45,7 +45,7 @@ namespace EvalOrder {
     }
     template <typename T> constexpr T &&b(T &&v) {
       if (!done_a)
-        throw "wrong"; // expected-note 5{{not valid}}
+        throw "wrong"; // expected-note 4{{not valid}}
       done_b = true;
       return (T &&)v;
     }
@@ -75,8 +75,7 @@ namespace EvalOrder {
   SEQ(A(&ud)->*B(&UserDefined::n));
 
   // Rule 4: a(b1, b2, b3)
-  SEQ(A(f)(B(1), B(2), B(3))); // expected-error {{not an integral constant expression}} FIXME \
-                               // expected-note 2{{in call to}}
+  SEQ(A(f)(B(1), B(2), B(3)));
 
   // Rule 5: b = a, b @= a
   SEQ(B(lvalue<int>().get()) = A(0)); // expected-error {{not an integral constant expression}} FIXME \

@tbaederr tbaederr merged commit f78d288 into llvm:main Aug 3, 2024
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 3, 2024

LLVM Buildbot has detected a new failure on builder clang-aarch64-sve-vls running on linaro-g3-02 while building clang at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/143/builds/1240

Here is the relevant piece of the build log for the reference:

Step 7 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'lit :: googletest-timeout.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 9
not env -u FILECHECK_OPTS "/usr/bin/python3.8" /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout    --param gtest_filter=InfiniteLoopSubTest --timeout=1 > /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out
# executed command: not env -u FILECHECK_OPTS /usr/bin/python3.8 /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --timeout=1
# .---command stderr------------
# | lit.py: /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# `-----------------------------
# RUN: at line 11
FileCheck --check-prefix=CHECK-INF < /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/googletest-timeout.py
# RUN: at line 16
not env -u FILECHECK_OPTS "/usr/bin/python3.8" /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout   --param gtest_filter=InfiniteLoopSubTest  --param set_timeout=1   > /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cfgset.out
# executed command: not env -u FILECHECK_OPTS /usr/bin/python3.8 /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --param set_timeout=1
# .---command stderr------------
# | Traceback (most recent call last):
# |   File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit/formats/googletest.py", line 304, in post_process_shard_results
# |     testsuites = json.load(f)["testsuites"]
# |   File "/usr/lib/python3.8/json/__init__.py", line 293, in load
# |     return loads(fp.read(),
# |   File "/usr/lib/python3.8/json/__init__.py", line 357, in loads
# |     return _default_decoder.decode(s)
# |   File "/usr/lib/python3.8/json/decoder.py", line 337, in decode
# |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
# |   File "/usr/lib/python3.8/json/decoder.py", line 355, in raw_decode
# |     raise JSONDecodeError("Expecting value", s, err.value) from None
# | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
# | 
# | During handling of the above exception, another exception occurred:
# | 
# | Traceback (most recent call last):
# |   File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit.py", line 6, in <module>
# |     main()
# |   File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit/main.py", line 130, in main
# |     selected_tests, discovered_tests = GoogleTest.post_process_shard_results(
# |   File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls/llvm/llvm/utils/lit/lit/formats/googletest.py", line 306, in post_process_shard_results
# |     raise RuntimeError(
# | RuntimeError: Failed to parse json file: /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py-googletest-timeout-4160003-1-2.json
# | 
# `-----------------------------
# RUN: at line 19
FileCheck --check-prefix=CHECK-INF < /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cfgset.out /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/googletest-timeout.py
# .---command stderr------------
# | /home/tcwg-buildbot/worker/clang-aarch64-sve-vls/stage1/utils/lit/tests/googletest-timeout.py:34:14: error: CHECK-INF: expected string not found in input
# | # CHECK-INF: Timed Out: 1
# |              ^
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants