Bug 84947 - UBSAN: ipcp_bits_lattice::meet_with(generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> >, unsigned int) ../../gcc/ipa-cp.c:1058
Summary: UBSAN: ipcp_bits_lattice::meet_with(generic_wide_int<fixed_wide_int_storage<1...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 8.0.1
: P3 normal
Target Milestone: ---
Assignee: Martin Jambor
URL:
Keywords: ice-on-invalid-code
Depends on:
Blocks: ubsan
  Show dependency treegraph
 
Reported: 2018-03-19 08:21 UTC by Martin Liška
Modified: 2018-04-06 13:20 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-03-19 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2018-03-19 08:21:08 UTC
Following causes UBSAN:

$ cat a.c
int main() { foo(0); }

$ cat b.c
struct a {
} foo(struct a b) {
}

$ gcc -flto -O2 [ab].c
a.c: In function ‘main’:
a.c:1:14: warning: implicit declaration of function ‘foo’ [-Wimplicit-function-declaration]
 int main() { foo(0); }
              ^~~
a.c:1:14: warning: type of ‘foo’ does not match original declaration [-Wlto-type-mismatch]
 int main() { foo(0); }
              ^
b.c:2:3: note: return value type mismatch
 } foo(struct a b) {
   ^
b.c:2:3: note: type ‘struct a’ should match type ‘int’
b.c:2:3: note: ‘foo’ was previously declared here
../../gcc/hwint.h:293:61: runtime error: shift exponent 64 is too large for 64-bit type 'long unsigned int'
    #0 0x37bd3d1 in sext_hwi ../../gcc/hwint.h:293
    #1 0x37bd3d1 in wi::binary_traits<generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> >, wi::int_traits<generic_wide_int<fixed_wide_int_storage<192> > >::precision_type, wi::int_traits<generic_wide_int<fixed_wide_int_storage<192> > >::precision_type>::result_type wi::sext<generic_wide_int<fixed_wide_int_storage<192> > >(generic_wide_int<fixed_wide_int_storage<192> > const&, unsigned int) ../../gcc/wide-int.h:2133
    #2 0x37bd3d1 in ipcp_bits_lattice::meet_with(generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> >, unsigned int) ../../gcc/ipa-cp.c:1058
    #3 0x37c3d07 in propagate_bits_across_jump_function(cgraph_edge*, int, ipa_jump_func*, ipcp_bits_lattice*) ../../gcc/ipa-cp.c:1878
    #4 0x37c5892 in propagate_constants_across_call ../../gcc/ipa-cp.c:2317
    #5 0x37d8ca2 in propagate_constants_topo ../../gcc/ipa-cp.c:3224
    #6 0x37d8ca2 in ipcp_propagate_stage ../../gcc/ipa-cp.c:3318
    #7 0x37e28ee in ipcp_driver ../../gcc/ipa-cp.c:5044
    #8 0x37e28ee in execute ../../gcc/ipa-cp.c:5138
    #9 0x16690a0 in execute_one_pass(opt_pass*) ../../gcc/passes.c:2497
    #10 0x166e35a in execute_ipa_pass_list(opt_pass*) ../../gcc/passes.c:2932
    #11 0x6a22f8 in do_whole_program_analysis ../../gcc/lto/lto.c:3147
    #12 0x6a22f8 in lto_main() ../../gcc/lto/lto.c:3368
    #13 0x1a607ca in compile_file ../../gcc/toplev.c:455
    #14 0x628cdc in do_compile ../../gcc/toplev.c:2132
    #15 0x628cdc in toplev::main(int, char**) ../../gcc/toplev.c:2267
    #16 0x62b76a in main ../../gcc/main.c:39
    #17 0x7ffff5cafa86 in __libc_start_main (/lib64/libc.so.6+0x21a86)
    #18 0x62b899 in _start (/home/marxin/bin/gcc/lib/gcc/x86_64-pc-linux-gnu/8.0.1/lto1+0x62b899)
Comment 1 Richard Biener 2018-03-19 11:04:09 UTC
#if defined (__GNUC__)
    {
      /* Take the faster path if the implementation-defined bits it's relying
         on are implemented the way we expect them to be.  Namely, conversion
         from unsigned to signed preserves bit pattern, and right shift of
         a signed value propagates the sign bit.
         We have to convert from signed to unsigned and back, because when left
         shifting signed values, any overflow is undefined behavior.  */
      gcc_checking_assert (prec < HOST_BITS_PER_WIDE_INT);
      int shift = HOST_BITS_PER_WIDE_INT - prec;
      return ((HOST_WIDE_INT) ((unsigned HOST_WIDE_INT) src << shift)) >> shift;
    }
#else

so prec is zero for some reason?
Comment 2 Martin Liška 2018-03-19 14:12:11 UTC
Because:

(gdb) p debug_tree(parm_type)
 <record_type 0x7ffff52be930 a BLK
    size <integer_cst 0x7ffff50c5c60 type <integer_type 0x7ffff50d90a8 bitsizetype> constant 0>
    unit-size <integer_cst 0x7ffff50c5c18 type <integer_type 0x7ffff50d9000 sizetype> constant 0>
    align:8 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type 0x7ffff52be930 context <translation_unit_decl 0x7ffff50cf1e0 b.c>
    chain <type_decl 0x7ffff52c0130 D.4324>>

In the original testcase it's also zero for:

$ gcc -flto /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/lto/20120723_*.c -O2

(gdb) p parm_type
$2 = (tree) 0x7ffff52be690
(gdb) p debug_tree(parm_type)
 <record_type 0x7ffff52be690 E DI
    size <integer_cst 0x7ffff50c5be8 type <integer_type 0x7ffff50d90a8 bitsizetype> constant 64>
    unit-size <integer_cst 0x7ffff50c5c00 type <integer_type 0x7ffff50d9000 sizetype> constant 8>
    align:64 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type 0x7ffff52be690
    fields <field_decl 0x7ffff52c05f0 p
        type <pointer_type 0x7ffff52be738 type <record_type 0x7ffff52be7e0 S>
            unsigned DI size <integer_cst 0x7ffff50c5be8 64> unit-size <integer_cst 0x7ffff50c5c00 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set 1 structural-equality
            pointer_to_this <pointer_type 0x7ffff52be348>>
        unsigned DI /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/lto/20120723_1.c:15:13 size <integer_cst 0x7ffff50c5be8 64> unit-size <integer_cst 0x7ffff50c5c00 8>
        align:64 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff50c5c18 constant 0>
        bit-offset <integer_cst 0x7ffff50c5c60 constant 0> context <record_type 0x7ffff52be690 E>> context <translation_unit_decl 0x7ffff50cf1e0 /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/lto/20120723_1.c>
    chain <type_decl 0x7ffff52c0688 D.4340>>
Comment 3 Richard Biener 2018-03-19 14:17:57 UTC
bool
propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
                                     ipa_jump_func *jfunc,
                                     ipcp_bits_lattice *dest_lattice)
{
...
  /* For K&R C programs, ipa_get_type() could return NULL_TREE.
     Avoid the transform for these cases.  */
  if (!parm_type)
    {
      if (dump_file && (dump_flags & TDF_DETAILS))
        fprintf (dump_file, "Setting dest_lattice to bottom, because"
                            " param %i type is NULL for %s\n", idx,
                            cs->callee->name ());

      return dest_lattice->set_to_bottom ();
    }

  unsigned precision = TYPE_PRECISION (parm_type);
  signop sgn = TYPE_SIGN (parm_type);

those two accesses only work for INTEGRAL_TYPE_P types.  So maybe
the !parm_type should be instead

   if (!parm_type || !INTEGRAL_TYPE_P (parm_type))

? (with appropriate adjustment of the dump message)
Comment 4 Richard Biener 2018-03-19 14:19:33 UTC
(In reply to Richard Biener from comment #3)
> bool
> propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
>                                      ipa_jump_func *jfunc,
>                                      ipcp_bits_lattice *dest_lattice)
> {
> ...
>   /* For K&R C programs, ipa_get_type() could return NULL_TREE.
>      Avoid the transform for these cases.  */
>   if (!parm_type)
>     {
>       if (dump_file && (dump_flags & TDF_DETAILS))
>         fprintf (dump_file, "Setting dest_lattice to bottom, because"
>                             " param %i type is NULL for %s\n", idx,
>                             cs->callee->name ());
> 
>       return dest_lattice->set_to_bottom ();
>     }
> 
>   unsigned precision = TYPE_PRECISION (parm_type);
>   signop sgn = TYPE_SIGN (parm_type);
> 
> those two accesses only work for INTEGRAL_TYPE_P types.  So maybe
> the !parm_type should be instead
> 
>    if (!parm_type || !INTEGRAL_TYPE_P (parm_type))

!(INTEGRAL_TYPE_P (parm_type) || POINTER_TYPE_P (parm_type))

of course.

> ? (with appropriate adjustment of the dump message)

Does IPA-CP in any way handle jump functions of float types?
Comment 5 Martin Liška 2018-03-19 14:25:08 UTC
I can confirm it works for me:

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index ee41a8d55b7..96fff686a3a 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1813,7 +1813,8 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
 
   /* For K&R C programs, ipa_get_type() could return NULL_TREE.
      Avoid the transform for these cases.  */
-  if (!parm_type)
+  if (!parm_type || !(INTEGRAL_TYPE_P (parm_type)
+		      || POINTER_TYPE_P (parm_type)))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file, "Setting dest_lattice to bottom, because"

I'm leaving that to Martin.
Comment 6 Martin Jambor 2018-03-29 13:21:34 UTC
(In reply to Richard Biener from comment #4)
> 
> Does IPA-CP in any way handle jump functions of float types?

It does not create any jump functions tracking individual bits, we
only create that part of jump functions for integral and pointer
types.

(In reply to Martin Liška from comment #5)
> I can confirm it works for me:
> 
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index ee41a8d55b7..96fff686a3a 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1813,7 +1813,8 @@ propagate_bits_across_jump_function (cgraph_edge *cs,
> int idx,
>  
>    /* For K&R C programs, ipa_get_type() could return NULL_TREE.
>       Avoid the transform for these cases.  */
> -  if (!parm_type)
> +  if (!parm_type || !(INTEGRAL_TYPE_P (parm_type)
> +		      || POINTER_TYPE_P (parm_type)))
>      {
>        if (dump_file && (dump_flags & TDF_DETAILS))
>  	fprintf (dump_file, "Setting dest_lattice to bottom, because"
> 
> I'm leaving that to Martin.

It seems exactly the right thing to do, let me just push that negation
into the parenthesis, bootstrap and submit to the mailing list.
Thanks.
Comment 7 Martin Jambor 2018-04-03 13:27:58 UTC
Author: jamborm
Date: Tue Apr  3 13:27:26 2018
New Revision: 259029

URL: https://gcc.gnu.org/viewcvs?rev=259029&root=gcc&view=rev
Log:
Bits propagation only for int and ptr types

2018-03-29  Martin Liska  <mliska@suse.cz>
	    Martin Jambor  <mjambor@suse.cz>

	PR ipa/84947
	* ipa-cp.c (propagate_bits_across_jump_function): Bail out if
	param_type is not an integral or pointer type.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-cp.c
Comment 8 Martin Jambor 2018-04-06 13:20:50 UTC
Fixed