On Sun, 6 Sep 2026, Rohith Kapelli wrote:
> func_checker::compare_operand compares non-memory operands with a bare
> operand_equal_p and OEP_MATCH_SIDE_EFFECTS. When the operand is an
> ADDR_EXPR, operand_equal_p recurses with OEP_ADDRESS_OF set, and for a
> COMPONENT_REF under OEP_ADDRESS_OF it accepts two different FIELD_DECLs
> whose DECL_FIELD_OFFSET and DECL_FIELD_BIT_OFFSET agree. That is correct
> for the value of the address, but ICF is not only comparing an address.
> The surviving body carries one of the two field types into every caller,
> and get_range_strlen_tree reads TYPE_DOMAIN and TYPE_MAX_VALUE off it.
>
> In the PR two structs hold a char array as their first member, one of 11
> and one of 128 elements, with identical operator== bodies. ICF unifies
> them, the surviving body carries char[11], and the strlen machinery then
> concludes that a 15 character string cannot be there.
>
> Pass OEP_ADDRESS_OF_SAME_FIELD, which tree-core.h already documents for
> this case: "For OEP_ADDRESS_OF of COMPONENT_REFs, only consider same
> fields as equivalent rather than also different fields with the same
> offset." fold-const.cc honours it at the point where the two offsets
> would otherwise be accepted.
>
> PR ipa/119006
>
> gcc/ChangeLog:
>
> * ipa-icf-gimple.cc (func_checker::compare_operand): Pass
> OEP_ADDRESS_OF_SAME_FIELD to operand_equal_p.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/ipa/pr119006.C: New test.
>
> Signed-off-by: Rohith Kapelli <[email protected]>
> ---
> PR119006 is wrong code, filed 2025-02-25 and live on gcc-13, gcc-14,
> gcc-15, gcc-16 and trunk. Two structs hold a char array as their first
> member, one char[11] and one char[128], with identical operator== bodies.
> ICF unifies the two operator==, the surviving body carries char[11], and
> get_range_strlen_tree reads TYPE_DOMAIN off that type and concludes that a
> 15 character string cannot be there. The comparison folds to a constant and
> the program takes the wrong branch.
>
> The mechanism is in func_checker::compare_operand. Non-memory operands go
> through a bare operand_equal_p with OEP_MATCH_SIDE_EFFECTS. For an
> ADDR_EXPR operand_equal_p recurses with OEP_ADDRESS_OF set, and for a
> COMPONENT_REF under OEP_ADDRESS_OF fold-const.cc accepts two different
> FIELD_DECLs when DECL_FIELD_OFFSET and DECL_FIELD_BIT_OFFSET agree. Both
> _str members sit at offset 0, so the fields compare equal and the functions
> are unified.
>
> Comment 10 asks for a flag that would check the MEM_REF details even under
> OEP_ADDRESS_OF. The divergence here is one step before that: field identity
> on the COMPONENT_REF at fold-const.cc:3599, not a MEM_REF detail.
>
> GCC already documents a flag for that fallback, on every branch this patch
> targets and gcc-13 included. tree-core.h:
>
> /* For OEP_ADDRESS_OF of COMPONENT_REFs, only consider same fields as
> equivalent rather than also different fields with the same offset. */
> OEP_ADDRESS_OF_SAME_FIELD = 256,
>
> It has one consumer, fold-const.cc:3599, and three callers, all of them in
> front ends: cp/call.cc, c-family/c-warn.cc and c/c-typeck.cc, none in the
> middle end. The patch passes it from compare_operand.
>
> Comment 12 asks how often this fires, and notes that merging semantically
> equivalent functions from different template instantiations is an important
> case of ICF, over 20 percent of LibreOffice code size.
>
> Here is that measurement, and the patch is not free. On
> libstdc++-v3/src/c++{98,11,17,20}/*.cc compiled to objects and linked as
> one shared library with LTO, 105 objects, it declines 25 merges at -O2 and
> 29 at -Os while gaining 3 at each level, so 25 of 255 and 29 of 270. Read
> off the assembler names in the WPA dump they are almost entirely char
> against wchar_t instantiations of stream and locale accessors, rdbuf 9,
> is_open 7, do_is_equal 4, operator= 2, fd and file 2, one transactional
> error message getter, and one pair from two different class templates
> sharing an accessor body. That is the cross-instantiation class comment 12
> calls important, and what it costs is 264 bytes of .text at -O2 and 2188 at
> -Os, against 1710731 and 1365540 bytes unpatched, so 0.015 and 0.16
> percent.
>
> An instrumented compiler that logs this divergence emitted 0 lines over the
> same workload at both levels, 525 merges. Its sensitivity was established
> first on four constructed divergence shapes and it fires on all four. That
> zero is the rate at which this bug fires on this workload, not a claim that
> ICF never merges across divergent metadata.
>
> The workload is one library, and libstdc++'s own sources are library
> implementation rather than application-scale template instantiation, so
> this speaks to comment 12's reference point without reaching it. No
> LibreOffice-scale C++ body was available here to measure.
>
> Comment 12 offers two routes, giving up the merge or modifying the merged
> body into a form safe for both. This patch takes the first. I prototyped
> the second, rewriting the diverging ADDR_EXPR to a MEM_REF and retyping the
> SSA name: it fixes all 40 configurations while keeping the merge, recovers
> trunk's 255 and 270 merges exactly, and bootstraps, but it fires zero times
> across those 525 merges, so every aggregate result validated it dormant and
> the finding rests on one instance. Feasibility, not a proposal.
>
> Testing, on aarch64-unknown-linux-gnu at trunk 95c85b404da, patched and
> unpatched from the same tree in the same environment with only
> ipa-icf-gimple.cc differing. Three stage bootstrap configured
> --enable-languages=c,c++,lto --enable-checking=yes,rtl,extra
> --disable-multilib --disable-nls --disable-libsanitizer, make compare
> reported comparison successful. Shard sums merged with
> contrib/dg-extract-results.sh:
>
> tool variant total PASS FAIL UNRESOLVED
> gcc trunk 408405 401230 71 0
> gcc patch 408405 401230 71 0
> g++ trunk 474714 468765 52 0
> g++ patch 474777 468828 52 0
>
> The gcc suite is identical line for line. The g++ totals differ by 63 test
> names, which is g++.dg/modules churn that appears between unpatched runs as
> well; no FAIL and no UNRESOLVED is added or removed.
>
> Runtime libraries, unpatched against patched, comparing sorted PASS, FAIL,
> XPASS and UNRESOLVED lines: libstdc++ 18914 PASS and 1 FAIL, libgomp 6273,
> libitm 44, libatomic 54, and 0 differing lines in all four. The libstdc++
> FAIL is experimental/net/internet/resolver/ops/lookup.cc, which needs DNS
> the test machine does not have, and is present unpatched.
>
> A 40 configuration reproducer matrix, 2 testcases by 5 optimisation levels
> by 4 flag sets, goes from 6 failing to 0. The same matrix on the release
> branches with the patch applied to each:
>
> gcc-16 bf726b75da7 6 failing before, 0 after
> gcc-15 ae79106ea90 6 failing before, 0 after
> gcc-14 8c5c1a3eca2 6 failing before, 0 after
> gcc-13 e1926c31dcb 6 failing before, 0 after
>
> On gcc-13 the hunk applies at offset -1 lines.
>
> The testcase is revert sensitive. One build tree, patch applied, then
> reverted in place with only the compiler rebuilt, then reapplied:
>
> applied 12 PASS, 0 FAIL
> reverted 3 PASS, 9 FAIL
> reapplied 12 PASS, 0 FAIL
>
> The 3 that pass unpatched are the excess errors lines, one per variant.
> Variants exercised are gnu++11, gnu++20 and gnu++29.
>
> On the choice: Jakub's comment 11 patch 1 also fixes this, by intercepting
> ADDR_EXPR and comparing the operands without OEP_ADDRESS_OF, which restores
> the same field rule and the MEM_REF checks too. On libstdc++ it declines
> the identical set and produces the same .text, and I could not separate the
> two on evidence: across three constructed shapes at four option
> combinations every alignment sensitive pair was rejected by compare_ao_refs
> before the blind operand_equal_p was reached, and three shapes is not a
> proof. I bootstrapped and regression tested his patch here too, with the
> same results, and the testcase is revert sensitive against it in the same
> way. If it is preferred, it is his and it is ready, the testing above
> applies to it unchanged, and I have no argument against it.
>
> Where would this fire at scale, since libstdc++ gives zero?
> Must such a rewrite work at WPA, given 8a917760aa4 takes a !flag_wpa exit?
> Is retyping an SSA name in place acceptable, or must consumers be checked?
>
> OK for trunk?
OK.
Thanks,
Richard.
> gcc/ipa-icf-gimple.cc | 3 ++-
> gcc/testsuite/g++.dg/ipa/pr119006.C | 33 +++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/ipa/pr119006.C
>
> diff --git a/gcc/ipa-icf-gimple.cc b/gcc/ipa-icf-gimple.cc
> index a893a3faaa4..720a3a0ce1a 100644
> --- a/gcc/ipa-icf-gimple.cc
> +++ b/gcc/ipa-icf-gimple.cc
> @@ -439,7 +439,8 @@ func_checker::compare_operand (tree t1, tree t2,
> operand_access_type access)
> }
> else
> {
> - if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS))
> + if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS
> + | OEP_ADDRESS_OF_SAME_FIELD))
> return true;
> return return_false_with_msg
> ("operand_equal_p failed");
> diff --git a/gcc/testsuite/g++.dg/ipa/pr119006.C
> b/gcc/testsuite/g++.dg/ipa/pr119006.C
> new file mode 100644
> index 00000000000..aa975eb3004
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr119006.C
> @@ -0,0 +1,33 @@
> +// PR ipa/119006
> +// { dg-do run { target c++11 } }
> +// { dg-options "-O2 -fwhole-program -fdump-ipa-icf-details" }
> +
> +struct FixedString10 {
> + bool operator==(const char* rhs_) const { return rhs_ and not
> __builtin_strcmp(_str, rhs_); }
> + char _str[11];
> +};
> +struct FixedString127 {
> + bool operator==(const char* rhs_) const { return rhs_ and not
> __builtin_strcmp(_str, rhs_); }
> + bool operator!=(const char* rhs_) const { return !(*this == rhs_); }
> + char _str[128];
> +};
> +[[gnu::noinline,gnu::used]]
> +int fixedStringUser(const FixedString10& lhs, const char* rhs) {
> + return lhs == rhs;
> +}
> +constexpr const char *t = "StrOverTenChars";
> +[[gnu::noinline,gnu::used]]
> +void checkString(FixedString127 reason_) {
> + if (reason_ != t) __builtin_trap();
> +}
> +
> +int main(int argc, char** argv) {
> + FixedString127 fs;
> + __builtin_strcpy(fs._str, t);
> + checkString(fs);
> +}
> +
> +// The two operator== bodies are identical but their _str members have
> +// different bounds, so ICF must not treat them as equal.
> +// { dg-final { scan-ipa-dump-not "Unified" "icf" } }
> +// { dg-final { scan-ipa-dump "Equal symbols: 0" "icf" } }
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
