On Tue, 2 Sep 2025, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, LOCATION_LINE is represented in an int,
> and while we have -pedantic diagnostics (and -pedantic-error error)
> for too large #line, we can still overflow into negative line
> numbers up to -2 and -1. We could overflow to that even with valid
> source if it says has #line 2147483640 and then just has
> 2G+ lines after it.
> Now, the ICE is because assign_discriminator{,s} uses a hash_map
> with int_hash <int64_t, -1, -2>, so values -2 and -1 are reserved
> for deleted and empty entries. We just need to make sure those aren't
> valid. One possible fix would be just that
> - discrim_entry &e = map.get_or_insert (LOCATION_LINE (loc), &existed);
> + discrim_entry &e
> + = map.get_or_insert ((unsigned) LOCATION_LINE (loc), &existed);
> by adding unsigned cast when the key is signed 64-bit, it will never
> be -1 or -2.
> But I think that is wasteful, discrim_entry is a struct with 2 unsigned
> non-static data members, so for lines which can only be 0 to 0xffffffff
> (sure, with wrap-around), I think just using a hash_map with 96bit elts
> is better than 128bit.
> So, the following patch just doesn't assign any discriminators for lines
> -1U and -2U, I think that is fine, normal programs never do that.
> Another possibility would be to handle lines -1U and -2U as if it was say
> -3U.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM.
Thanks,
Richard.
> 2025-09-02 Jakub Jelinek <[email protected]>
>
> PR middle-end/121663
> * tree-cfg.cc (assign_discriminator): Change map argument type
> from hash_map with int_hash <int64_t, -1, -2> to one with
> int_hash <unsigned, -1U, -2U>. Cast LOCATION_LINE to unsigned.
> Return early for (unsigned) LOCATION_LINE above -3U.
> (assign_discriminators): Change map type from hash_map with
> int_hash <int64_t, -1, -2> to one with int_hash <unsigned, -1U, -2U>.
>
> * gcc.dg/pr121663.c: New test.
>
> --- gcc/tree-cfg.cc.jj 2025-08-23 15:00:04.928779140 +0200
> +++ gcc/tree-cfg.cc 2025-09-01 15:56:17.711505423 +0200
> @@ -1095,10 +1095,14 @@ struct discrim_entry
>
> location_t
> assign_discriminator (location_t loc, unsigned int bb_id,
> - hash_map<int_hash <int64_t, -1, -2>, discrim_entry> &map)
> + hash_map<int_hash <unsigned, -1U, -2U>,
> + discrim_entry> &map)
> {
> bool existed;
> - discrim_entry &e = map.get_or_insert (LOCATION_LINE (loc), &existed);
> + if ((unsigned) LOCATION_LINE (loc) >= -2U)
> + return loc;
> + discrim_entry &e
> + = map.get_or_insert ((unsigned) LOCATION_LINE (loc), &existed);
> gcc_checking_assert (!has_discriminator (loc));
> if (!existed)
> {
> @@ -1121,7 +1125,7 @@ assign_discriminator (location_t loc, un
> static void
> assign_discriminators (void)
> {
> - hash_map<int_hash <int64_t, -1, -2>, discrim_entry> map (13);
> + hash_map<int_hash <unsigned, -1U, -2U>, discrim_entry> map (13);
> unsigned int bb_id = 0;
> basic_block bb;
> FOR_EACH_BB_FN (bb, cfun)
> --- gcc/testsuite/gcc.dg/pr121663.c.jj 2025-09-01 16:09:17.623363163
> +0200
> +++ gcc/testsuite/gcc.dg/pr121663.c 2025-09-01 16:09:41.325054619 +0200
> @@ -0,0 +1,9 @@
> +/* PR middle-end/121663 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +void
> +foo (void)
> +{
> +#line 4294967295
> +}
>
> Jakub
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
