Author: Pierre van Houtryve Date: 2023-01-06T02:57:50-05:00 New Revision: d6acd0196b3378bdeb5193053e290d7194c4f72d
URL: https://github.com/llvm/llvm-project/commit/d6acd0196b3378bdeb5193053e290d7194c4f72d DIFF: https://github.com/llvm/llvm-project/commit/d6acd0196b3378bdeb5193053e290d7194c4f72d.diff LOG: [Sema] Fix crash when evaluating nested call with value-dependent arg Fix an edge case `ExprConstant.cpp`'s `EvaluateWithSubstitution` when called by `CheckEnableIf` The assertion in `CallStackFrame::getTemporary` could fail during evaluation of nested calls to a function using `enable_if` when the second argument was a value-dependent expression. This caused a temporary to be created for the second argument with a given version during the evaluation of the inner call, but we bailed out when evaluating the second argument of the outer call due to the expression being value-dependent. After bailing out, we tried to clean up the argument's value slot but it caused an assertion to trigger in `getTemporary` as a temporary for the second argument existed, but only for the inner call and not the outer call. See the test case for a more complete description of the issue. Reviewed By: ahatanak Differential Revision: https://reviews.llvm.org/D139713 Added: clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp Modified: clang/lib/AST/ExprConstant.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 78cfecbec9fd3..a43845e53c5d0 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -594,11 +594,6 @@ namespace { auto LB = Temporaries.lower_bound(KV); if (LB != Temporaries.end() && LB->first == KV) return &LB->second; - // Pair (Key,Version) wasn't found in the map. Check that no elements - // in the map have 'Key' as their key. - assert((LB == Temporaries.end() || LB->first.first != Key) && - (LB == Temporaries.begin() || std::prev(LB)->first.first != Key) && - "Element with key 'Key' found in map"); return nullptr; } diff --git a/clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp b/clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp new file mode 100644 index 0000000000000..998f2ccf92534 --- /dev/null +++ b/clang/test/SemaCXX/enable_if-nested-call-with-valuedependent-param.cpp @@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -fsyntax-only %s -std=c++14 + +// Checks that Clang doesn't crash/assert on the nested call to "kaboom" +// in "bar()". +// +// This is an interesting test case for `ExprConstant.cpp`'s `CallStackFrame` +// because it triggers the following chain of events: +// 0. `CheckEnableIf` calls `EvaluateWithSubstitution`. +// 1. The outer call to "kaboom" gets evaluated. +// 2. The expr for "a" gets evaluated, it has a version X; +// a temporary with the key (a, X) is created. +// 3. The inner call to "kaboom" gets evaluated. +// 4. The expr for "a" gets evaluated, it has a version Y; +// a temporary with the key (a, Y) is created. +// 5. The expr for "b" gets evaluated, it has a version Y; +// a temporary with the key (b, Y) is created. +// 6. `EvaluateWithSubstitution` looks at "b" but cannot evaluate it +// because it's value-dependent (due to the call to "f.foo()"). +// +// When `EvaluateWithSubstitution` bails out while evaluating the outer +// call, it attempts to fetch "b"'s param slot to clean it up. +// +// This used to cause an assertion failure in `getTemporary` because +// a temporary with the key "(b, Y)" (created at step 4) existed but +// not one for "(b, X)", which is what it was trying to fetch. + +template<typename T> +__attribute__((enable_if(true, ""))) +T kaboom(T a, T b) { + return b; +} + +struct A { + double foo(); +}; + +template <int> +struct B { + A &f; + + void bar() { + kaboom(kaboom(0.0, 1.0), f.foo()); + } +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
