Record function argument evaluation order in configt::ansi_ct#9120
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new architectural configuration parameter to CBMC’s configt::ansi_ct to record the target/compiler’s function-argument evaluation order, persists it into the architecture symbols (__CPROVER_architecture_*) in goto binaries, and restores it when loading those binaries (with backward compatibility for older binaries).
Changes:
- Introduce
configt::ansi_ct::argument_evaluation_orderand set it per architecture + compiler flavour (notably x86-family GCC/MSVC vs others). - Emit the parameter as
__CPROVER_architecture_argument_evaluation_orderin internal additions, and restore it from the symbol table when reading goto binaries. - Add unit + regression tests to validate the architecture/flavour matrix and symbol-table plumbing.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
unit/util/argument_evaluation_order.cpp |
New unit test validating evaluation order across architecture/flavour combinations. |
unit/Makefile |
Adds the new unit test file to the unit build (Makefile path). |
src/util/config.h |
Defines the new argument_evaluation_order enum/field on configt::ansi_ct. |
src/util/config.cpp |
Sets defaults/overrides per arch; restores from symbol table if present. |
src/ansi-c/ansi_c_internal_additions.cpp |
Emits the new architecture symbol into the internal additions. |
regression/cbmc/architecture_argument_evaluation_order/test.desc |
Regression test ensuring the symbol appears in --show-symbol-table. |
regression/cbmc/architecture_argument_evaluation_order/main.c |
Minimal input program for the regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d4e5cfe to
0a79951
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9120 +/- ##
===========================================
+ Coverage 80.77% 80.83% +0.05%
===========================================
Files 1712 1715 +3
Lines 189833 189948 +115
Branches 73 73
===========================================
+ Hits 153347 153537 +190
+ Misses 36486 36411 -75 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ab66e45 to
04d1bb7
Compare
The C and C++ standards leave the order of evaluation of function call arguments unspecified, but any given compiler/architecture combination uses a fixed order, which is observable when argument expressions have side effects. Empirically (test programs on native and cross-compiled targets, and via Compiler Explorer): GCC evaluates right-to-left on the x86 family (i386, x86_64, x32) and left-to-right on all other architectures tested (arm64, arm, riscv64, ppc64le, mips64el, s390x, sparc64); Visual Studio evaluates right-to-left on all architectures tested (x86, x64, arm64); Clang evaluates left-to-right on all architectures tested (x86_64, arm64). Record this architectural parameter in configt::ansi_ct, setting it in each set_arch_spec_* function like the other architectural parameters (some of those functions are invoked directly by goto-cc's handling of -m16/-m32/-mthumb and friends, bypassing set_arch), and re-deriving it in goto-cc when the compiler flavour is switched after the architecture has been configured (goto-clang and goto-cl set the mode only after config.set). Expose it as __CPROVER_architecture_argument_evaluation_order, and restore it when loading goto binaries (tolerating older binaries that lack the symbol). Use the parameter during goto conversion when lowering side effects in function call arguments, which were previously always evaluated left-to-right, closing the 'TODO: evaluation order' in clean_expr. All lowering paths for function calls (subexpressions, plain assignment statements via convert_assign, and do_function_call) share a helper so that the modelled order does not depend on the syntactic context of the call. The pre-existing test Function_Eval_Order2, which documented the fixed left-to-right lowering, now pins an architecture that evaluates left-to-right; new regression tests check both evaluation orders and both lowering paths using preprocessed input files so that no target-specific flags reach the native preprocessor on any CI host. The practical relevance of this parameter was demonstrated while investigating cross-platform proof-time instability (issue diffblue#8991): Z3 built with GCC on x86_64 and aarch64 takes different solver paths on byte-identical input because AST node identifiers are assigned in argument evaluation order (fixed upstream in Z3Prover/z3#10165). Co-authored-by: Kiro <[email protected]>
04d1bb7 to
26c7414
Compare
As suggested by @kroening in #8991 (comment): record the target's function argument evaluation order as an architectural parameter in
configt::ansi_ct— and use it during goto conversion.The C and C++ standards leave the order of evaluation of function call arguments unspecified, but any given compiler/architecture combination uses a fixed order, observable when argument expressions have side effects. Empirically verified (test programs on native x86_64 and aarch64 hosts, cross-compilation + qemu-user, and Compiler Explorer for MSVC and gcc
-mx32):Changes:
New architectural parameter
configt::ansi_ct::argument_evaluation_order, set in eachset_arch_spec_*function (matching the pattern of the other architectural parameters — important because goto-cc's-m16/-m32/-mthumbhandling invokes those functions directly, bypassingset_arch) and re-derived when goto-cc switches the compiler flavour after configuring the architecture (goto-clang/goto-cl). Exposed as__CPROVER_architecture_argument_evaluation_order, and restored when loading goto binaries (tolerating older binaries that lack the symbol, following the precedent of theosentry).Goto conversion now honours the parameter when lowering side effects in function call arguments, closing the long-standing
// TODO: evaluation orderinclean_expr. Previously arguments were always evaluated left-to-right, mis-modelling e.g. gcc on x86_64. All function-call lowering paths (subexpressions, plain assignment statements,do_function_call) share one helper, so the modelled order does not depend on the syntactic context of the call.The pre-existing test
Function_Eval_Order2, which documented the fixed left-to-right lowering, now pins an architecture where that order is real (--arch arm64); new regression tests cover both orders. These tests use preprocessed (.i) inputs so that no target-specific flags reach the native preprocessor on any CI host (pinning--archwith.cinputs breaks on the ARM runners, cf. the first CI iteration of this PR).Unit tests cover the architecture/flavour matrix and the goto-binary restore path (both the symbol-present and the old-binary branch).
Each commit message has a non-empty body, explaining why the change was made.
Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
n/a My commit message includes data points confirming performance improvements (if claimed).
My PR is restricted to a single feature or bugfix.
n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.