Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-20 Thread Alexei Starovoitov
On Sat, Jan 18, 2014 at 3:31 AM, Dorau, Lukasz  wrote:
> On Friday, January 17, 2014 10:44 PM Alexei Starovoitov 
>  wrote:
>> On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
>>  wrote:
>> > On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
>> >> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
>> >>  wrote:
>> >> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz 
>> wrote:
>> >> >> Hi
>> >> >>
>> >> >> My story is very simply...
>> >> >> I applied the following patch:
>> >> >>
>> >> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> >> >> --- a/drivers/scsi/isci/init.c
>> >> >> +++ b/drivers/scsi/isci/init.c
>> >> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
>> >> >> const
>> struct pci_device_id *id)
>> >> >> if (err)
>> >> >> goto err_host_alloc;
>> >> >>
>> >> >> -   for_each_isci_host(i, isci_host, pdev)
>> >> >> +   for_each_isci_host(i, isci_host, pdev) {
>> >> >> +   pr_err("(%d < %d) == %d\n",\
>> >> >> +  i, SCI_MAX_CONTROLLERS, (i < 
>> >> >> SCI_MAX_CONTROLLERS));
>> >> >>         scsi_scan_host(to_shost(isci_host));
>> >> >> +   }
>> >> >>
>> >> >> return 0;
>> >> >>
>> >> >> --
>> >> >> 1.8.3.1
>> >> >>
>> >> >> Then I issued the command 'modprobe isci' on platform with two SCU
>> controllers (Patsburg D or T chipset)
>> >> >> and received the following, very strange, output:
>> >> >>
>> >> >> (0 < 2) == 1
>> >> >> (1 < 2) == 1
>> >> >> (2 < 2) == 1
>> >> >>
>> >> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>> >> >
>> >> > gcc sees that i < array_size is the same as i < 2 as part of loop 
>> >> > condition, so
>> >> > it optimizes (i < sci_max_controllers) into constant 1.
>> >> > and emits printk like:
>> >> >   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>> >> >
>> >> >> (The kernel was compiled using gcc version 4.8.2.)
>> >> >
>> >> > it actually looks to be gcc 4.8 bug.
>> >> > Can you try gcc 4.7 ?
>> >> >
>> >>
>> >> It is interesting GCC 4.8 bug,
>> >> since it seems to expose issues in two compiler passes.
>> >>
>> >> here is test case:
>> >>
>> >> struct isci_host;
>> >> struct isci_orom;
>> >>
>> >> struct isci_pci_info {
>> >>   struct isci_host *hosts[2];
>> >>   struct isci_orom *orom;
>> >> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
>> >>
>> >> int printf(const char *fmt, ...);
>> >>
>> >> int isci_pci_probe()
>> >> {
>> >>   int i;
>> >>   struct isci_host *isci_host;
>> >>
>> >>   for (i = 0, isci_host = v.hosts[i];
>> >>i < 2 && isci_host;
>> >>isci_host = v.hosts[++i]) {
>> >> printf("(%d < %d) == %d\n", i, 2, (i < 2));
>> >>   }
>> >>
>> >>   return 0;
>> >> }
>> >>
>> >> int main()
>> >> {
>> >>   isci_pci_probe();
>> >> }
>> >>
>> >> $ gcc bug.c
>> >> $./a.out
>> >> 0 < 2) == 1
>> >> (1 < 2) == 1
>> >> $ gcc bug.c -O2
>> >> $ ./a.out
>> >> (0 < 2) == 1
>> >> (1 < 2) == 1
>> >> Segmentation fault (core dumped)
>> >
>> > Your testcase is invalid:
>> >
>> > markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
>> > markus@x4 tmp % ./a.out
>> > (0 < 2) == 1
>> > (1 < 2) == 1
>> > bug.c:16:20: runtime error: index 2 out of bounds for type 'struct 
>> > isci_host *[2]'
>> >
>> > As Jakub Jelinek said on IRC, changing the loop to e.g.:
>> >
>> >   for (i = 0;
>> >i < 2 && (isci_host = v.hosts[i]);
>> >i++) {
>> >
>> > fixes the issue.
>>
>> testcase was reduced from drivers/scsi/isci/host.h written back in
>> 2011 (commit b329aff107)
>> #define for_each_isci_host(id, ihost, pdev) \
>> for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
>>  id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
>>  ihost = to_pci_info(pdev)->hosts[++id])
>>
>> yes, it does access 3rd element of 2 element array and will read junk.
>>
>> C standard says the behavior is undefined and comes handy in compiler 
>> defense,
>> but in this case compiler has obviously all the information to make
>> right decision
>> instead of misoptimizing the code.
>> So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
>> --
>
> Thank you for explanation!
>
> Alexei,
>
> Will you file a gcc bug for this case?

sure. filed for the record:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59892
Closed as invalid by Markus already.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-20 Thread Alexei Starovoitov
On Sat, Jan 18, 2014 at 3:31 AM, Dorau, Lukasz lukasz.do...@intel.com wrote:
 On Friday, January 17, 2014 10:44 PM Alexei Starovoitov 
 alexei.starovoi...@gmail.com wrote:
 On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
 mar...@trippelsdorf.de wrote:
  On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
  On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
  alexei.starovoi...@gmail.com wrote:
   On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com
 wrote:
   Hi
  
   My story is very simply...
   I applied the following patch:
  
   diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
   --- a/drivers/scsi/isci/init.c
   +++ b/drivers/scsi/isci/init.c
   @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
   const
 struct pci_device_id *id)
   if (err)
   goto err_host_alloc;
  
   -   for_each_isci_host(i, isci_host, pdev)
   +   for_each_isci_host(i, isci_host, pdev) {
   +   pr_err((%d  %d) == %d\n,\
   +  i, SCI_MAX_CONTROLLERS, (i  
   SCI_MAX_CONTROLLERS));
   scsi_scan_host(to_shost(isci_host));
   +   }
  
   return 0;
  
   --
   1.8.3.1
  
   Then I issued the command 'modprobe isci' on platform with two SCU
 controllers (Patsburg D or T chipset)
   and received the following, very strange, output:
  
   (0  2) == 1
   (1  2) == 1
   (2  2) == 1
  
   Can anyone explain why (2  2) is true? Is it a gcc bug?
  
   gcc sees that i  array_size is the same as i  2 as part of loop 
   condition, so
   it optimizes (i  sci_max_controllers) into constant 1.
   and emits printk like:
 printk (\13(%d  %d) == %d\n, i_382, 2, 1);
  
   (The kernel was compiled using gcc version 4.8.2.)
  
   it actually looks to be gcc 4.8 bug.
   Can you try gcc 4.7 ?
  
 
  It is interesting GCC 4.8 bug,
  since it seems to expose issues in two compiler passes.
 
  here is test case:
 
  struct isci_host;
  struct isci_orom;
 
  struct isci_pci_info {
struct isci_host *hosts[2];
struct isci_orom *orom;
  } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
 
  int printf(const char *fmt, ...);
 
  int isci_pci_probe()
  {
int i;
struct isci_host *isci_host;
 
for (i = 0, isci_host = v.hosts[i];
 i  2  isci_host;
 isci_host = v.hosts[++i]) {
  printf((%d  %d) == %d\n, i, 2, (i  2));
}
 
return 0;
  }
 
  int main()
  {
isci_pci_probe();
  }
 
  $ gcc bug.c
  $./a.out
  0  2) == 1
  (1  2) == 1
  $ gcc bug.c -O2
  $ ./a.out
  (0  2) == 1
  (1  2) == 1
  Segmentation fault (core dumped)
 
  Your testcase is invalid:
 
  markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
  markus@x4 tmp % ./a.out
  (0  2) == 1
  (1  2) == 1
  bug.c:16:20: runtime error: index 2 out of bounds for type 'struct 
  isci_host *[2]'
 
  As Jakub Jelinek said on IRC, changing the loop to e.g.:
 
for (i = 0;
 i  2  (isci_host = v.hosts[i]);
 i++) {
 
  fixes the issue.

 testcase was reduced from drivers/scsi/isci/host.h written back in
 2011 (commit b329aff107)
 #define for_each_isci_host(id, ihost, pdev) \
 for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
  id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
  ihost = to_pci_info(pdev)-hosts[++id])

 yes, it does access 3rd element of 2 element array and will read junk.

 C standard says the behavior is undefined and comes handy in compiler 
 defense,
 but in this case compiler has obviously all the information to make
 right decision
 instead of misoptimizing the code.
 So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
 --

 Thank you for explanation!

 Alexei,

 Will you file a gcc bug for this case?

sure. filed for the record:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59892
Closed as invalid by Markus already.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 < 2) true? Is it a gcc bug?

2014-01-18 Thread Dorau, Lukasz
On Friday, January 17, 2014 10:44 PM Alexei Starovoitov 
 wrote:
> On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
>  wrote:
> > On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
> >> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
> >>  wrote:
> >> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz 
> wrote:
> >> >> Hi
> >> >>
> >> >> My story is very simply...
> >> >> I applied the following patch:
> >> >>
> >> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> >> >> --- a/drivers/scsi/isci/init.c
> >> >> +++ b/drivers/scsi/isci/init.c
> >> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
> >> >> const
> struct pci_device_id *id)
> >> >> if (err)
> >> >> goto err_host_alloc;
> >> >>
> >> >> -   for_each_isci_host(i, isci_host, pdev)
> >> >> +   for_each_isci_host(i, isci_host, pdev) {
> >> >> +   pr_err("(%d < %d) == %d\n",\
> >> >> +  i, SCI_MAX_CONTROLLERS, (i < 
> >> >> SCI_MAX_CONTROLLERS));
> >> >> scsi_scan_host(to_shost(isci_host));
> >> >> +       }
> >> >>
> >> >> return 0;
> >> >>
> >> >> --
> >> >> 1.8.3.1
> >> >>
> >> >> Then I issued the command 'modprobe isci' on platform with two SCU
> controllers (Patsburg D or T chipset)
> >> >> and received the following, very strange, output:
> >> >>
> >> >> (0 < 2) == 1
> >> >> (1 < 2) == 1
> >> >> (2 < 2) == 1
> >> >>
> >> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
> >> >
> >> > gcc sees that i < array_size is the same as i < 2 as part of loop 
> >> > condition, so
> >> > it optimizes (i < sci_max_controllers) into constant 1.
> >> > and emits printk like:
> >> >   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> >> >
> >> >> (The kernel was compiled using gcc version 4.8.2.)
> >> >
> >> > it actually looks to be gcc 4.8 bug.
> >> > Can you try gcc 4.7 ?
> >> >
> >>
> >> It is interesting GCC 4.8 bug,
> >> since it seems to expose issues in two compiler passes.
> >>
> >> here is test case:
> >>
> >> struct isci_host;
> >> struct isci_orom;
> >>
> >> struct isci_pci_info {
> >>   struct isci_host *hosts[2];
> >>   struct isci_orom *orom;
> >> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
> >>
> >> int printf(const char *fmt, ...);
> >>
> >> int isci_pci_probe()
> >> {
> >>   int i;
> >>   struct isci_host *isci_host;
> >>
> >>   for (i = 0, isci_host = v.hosts[i];
> >>i < 2 && isci_host;
> >>isci_host = v.hosts[++i]) {
> >> printf("(%d < %d) == %d\n", i, 2, (i < 2));
> >>   }
> >>
> >>   return 0;
> >> }
> >>
> >> int main()
> >> {
> >>   isci_pci_probe();
> >> }
> >>
> >> $ gcc bug.c
> >> $./a.out
> >> 0 < 2) == 1
> >> (1 < 2) == 1
> >> $ gcc bug.c -O2
> >> $ ./a.out
> >> (0 < 2) == 1
> >> (1 < 2) == 1
> >> Segmentation fault (core dumped)
> >
> > Your testcase is invalid:
> >
> > markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
> > markus@x4 tmp % ./a.out
> > (0 < 2) == 1
> > (1 < 2) == 1
> > bug.c:16:20: runtime error: index 2 out of bounds for type 'struct 
> > isci_host *[2]'
> >
> > As Jakub Jelinek said on IRC, changing the loop to e.g.:
> >
> >   for (i = 0;
> >i < 2 && (isci_host = v.hosts[i]);
> >i++) {
> >
> > fixes the issue.
> 
> testcase was reduced from drivers/scsi/isci/host.h written back in
> 2011 (commit b329aff107)
> #define for_each_isci_host(id, ihost, pdev) \
> for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
>  id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
>  ihost = to_pci_info(pdev)->hosts[++id])
> 
> yes, it does access 3rd element of 2 element array and will read junk.
> 
> C standard says the behavior is undefined and comes handy in compiler defense,
> but in this case compiler has obviously all the information to make
> right decision
> instead of misoptimizing the code.
> So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
> --

Thank you for explanation!

Alexei,

Will you file a gcc bug for this case?

Thanks,
Lukasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 2) true? Is it a gcc bug?

2014-01-18 Thread Dorau, Lukasz
On Friday, January 17, 2014 10:44 PM Alexei Starovoitov 
alexei.starovoi...@gmail.com wrote:
 On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
 mar...@trippelsdorf.de wrote:
  On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
  On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
  alexei.starovoi...@gmail.com wrote:
   On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com
 wrote:
   Hi
  
   My story is very simply...
   I applied the following patch:
  
   diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
   --- a/drivers/scsi/isci/init.c
   +++ b/drivers/scsi/isci/init.c
   @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
   const
 struct pci_device_id *id)
   if (err)
   goto err_host_alloc;
  
   -   for_each_isci_host(i, isci_host, pdev)
   +   for_each_isci_host(i, isci_host, pdev) {
   +   pr_err((%d  %d) == %d\n,\
   +  i, SCI_MAX_CONTROLLERS, (i  
   SCI_MAX_CONTROLLERS));
   scsi_scan_host(to_shost(isci_host));
   +   }
  
   return 0;
  
   --
   1.8.3.1
  
   Then I issued the command 'modprobe isci' on platform with two SCU
 controllers (Patsburg D or T chipset)
   and received the following, very strange, output:
  
   (0  2) == 1
   (1  2) == 1
   (2  2) == 1
  
   Can anyone explain why (2  2) is true? Is it a gcc bug?
  
   gcc sees that i  array_size is the same as i  2 as part of loop 
   condition, so
   it optimizes (i  sci_max_controllers) into constant 1.
   and emits printk like:
 printk (\13(%d  %d) == %d\n, i_382, 2, 1);
  
   (The kernel was compiled using gcc version 4.8.2.)
  
   it actually looks to be gcc 4.8 bug.
   Can you try gcc 4.7 ?
  
 
  It is interesting GCC 4.8 bug,
  since it seems to expose issues in two compiler passes.
 
  here is test case:
 
  struct isci_host;
  struct isci_orom;
 
  struct isci_pci_info {
struct isci_host *hosts[2];
struct isci_orom *orom;
  } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
 
  int printf(const char *fmt, ...);
 
  int isci_pci_probe()
  {
int i;
struct isci_host *isci_host;
 
for (i = 0, isci_host = v.hosts[i];
 i  2  isci_host;
 isci_host = v.hosts[++i]) {
  printf((%d  %d) == %d\n, i, 2, (i  2));
}
 
return 0;
  }
 
  int main()
  {
isci_pci_probe();
  }
 
  $ gcc bug.c
  $./a.out
  0  2) == 1
  (1  2) == 1
  $ gcc bug.c -O2
  $ ./a.out
  (0  2) == 1
  (1  2) == 1
  Segmentation fault (core dumped)
 
  Your testcase is invalid:
 
  markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
  markus@x4 tmp % ./a.out
  (0  2) == 1
  (1  2) == 1
  bug.c:16:20: runtime error: index 2 out of bounds for type 'struct 
  isci_host *[2]'
 
  As Jakub Jelinek said on IRC, changing the loop to e.g.:
 
for (i = 0;
 i  2  (isci_host = v.hosts[i]);
 i++) {
 
  fixes the issue.
 
 testcase was reduced from drivers/scsi/isci/host.h written back in
 2011 (commit b329aff107)
 #define for_each_isci_host(id, ihost, pdev) \
 for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
  id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
  ihost = to_pci_info(pdev)-hosts[++id])
 
 yes, it does access 3rd element of 2 element array and will read junk.
 
 C standard says the behavior is undefined and comes handy in compiler defense,
 but in this case compiler has obviously all the information to make
 right decision
 instead of misoptimizing the code.
 So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
 --

Thank you for explanation!

Alexei,

Will you file a gcc bug for this case?

Thanks,
Lukasz

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
 wrote:
> On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
>> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
>>  wrote:
>> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz  
>> > wrote:
>> >> Hi
>> >>
>> >> My story is very simply...
>> >> I applied the following patch:
>> >>
>> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> >> --- a/drivers/scsi/isci/init.c
>> >> +++ b/drivers/scsi/isci/init.c
>> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
>> >> const struct pci_device_id *id)
>> >> if (err)
>> >> goto err_host_alloc;
>> >>
>> >> -   for_each_isci_host(i, isci_host, pdev)
>> >> +   for_each_isci_host(i, isci_host, pdev) {
>> >> +   pr_err("(%d < %d) == %d\n",\
>> >> +  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>> >> scsi_scan_host(to_shost(isci_host));
>> >> +   }
>> >>
>> >> return 0;
>> >>
>> >> --
>> >> 1.8.3.1
>> >>
>> >> Then I issued the command 'modprobe isci' on platform with two SCU 
>> >> controllers (Patsburg D or T chipset)
>> >> and received the following, very strange, output:
>> >>
>> >> (0 < 2) == 1
>> >> (1 < 2) == 1
>> >> (2 < 2) == 1
>> >>
>> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>> >
>> > gcc sees that i < array_size is the same as i < 2 as part of loop 
>> > condition, so
>> > it optimizes (i < sci_max_controllers) into constant 1.
>> > and emits printk like:
>> >   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>> >
>> >> (The kernel was compiled using gcc version 4.8.2.)
>> >
>> > it actually looks to be gcc 4.8 bug.
>> > Can you try gcc 4.7 ?
>> >
>>
>> It is interesting GCC 4.8 bug,
>> since it seems to expose issues in two compiler passes.
>>
>> here is test case:
>>
>> struct isci_host;
>> struct isci_orom;
>>
>> struct isci_pci_info {
>>   struct isci_host *hosts[2];
>>   struct isci_orom *orom;
>> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
>>
>> int printf(const char *fmt, ...);
>>
>> int isci_pci_probe()
>> {
>>   int i;
>>   struct isci_host *isci_host;
>>
>>   for (i = 0, isci_host = v.hosts[i];
>>i < 2 && isci_host;
>>isci_host = v.hosts[++i]) {
>> printf("(%d < %d) == %d\n", i, 2, (i < 2));
>>   }
>>
>>   return 0;
>> }
>>
>> int main()
>> {
>>   isci_pci_probe();
>> }
>>
>> $ gcc bug.c
>> $./a.out
>> 0 < 2) == 1
>> (1 < 2) == 1
>> $ gcc bug.c -O2
>> $ ./a.out
>> (0 < 2) == 1
>> (1 < 2) == 1
>> Segmentation fault (core dumped)
>
> Your testcase is invalid:
>
> markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
> markus@x4 tmp % ./a.out
> (0 < 2) == 1
> (1 < 2) == 1
> bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host 
> *[2]'
>
> As Jakub Jelinek said on IRC, changing the loop to e.g.:
>
>   for (i = 0;
>i < 2 && (isci_host = v.hosts[i]);
>i++) {
>
> fixes the issue.

testcase was reduced from drivers/scsi/isci/host.h written back in
2011 (commit b329aff107)
#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
 id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
 ihost = to_pci_info(pdev)->hosts[++id])

yes, it does access 3rd element of 2 element array and will read junk.

C standard says the behavior is undefined and comes handy in compiler defense,
but in this case compiler has obviously all the information to make
right decision
instead of misoptimizing the code.
So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Markus Trippelsdorf
On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
> On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
>  wrote:
> > On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz  
> > wrote:
> >> Hi
> >>
> >> My story is very simply...
> >> I applied the following patch:
> >>
> >> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> >> --- a/drivers/scsi/isci/init.c
> >> +++ b/drivers/scsi/isci/init.c
> >> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
> >> struct pci_device_id *id)
> >> if (err)
> >> goto err_host_alloc;
> >>
> >> -   for_each_isci_host(i, isci_host, pdev)
> >> +   for_each_isci_host(i, isci_host, pdev) {
> >> +   pr_err("(%d < %d) == %d\n",\
> >> +  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> >> scsi_scan_host(to_shost(isci_host));
> >> +   }
> >>
> >> return 0;
> >>
> >> --
> >> 1.8.3.1
> >>
> >> Then I issued the command 'modprobe isci' on platform with two SCU 
> >> controllers (Patsburg D or T chipset)
> >> and received the following, very strange, output:
> >>
> >> (0 < 2) == 1
> >> (1 < 2) == 1
> >> (2 < 2) == 1
> >>
> >> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
> >
> > gcc sees that i < array_size is the same as i < 2 as part of loop 
> > condition, so
> > it optimizes (i < sci_max_controllers) into constant 1.
> > and emits printk like:
> >   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
> >
> >> (The kernel was compiled using gcc version 4.8.2.)
> >
> > it actually looks to be gcc 4.8 bug.
> > Can you try gcc 4.7 ?
> >
> 
> It is interesting GCC 4.8 bug,
> since it seems to expose issues in two compiler passes.
> 
> here is test case:
> 
> struct isci_host;
> struct isci_orom;
> 
> struct isci_pci_info {
>   struct isci_host *hosts[2];
>   struct isci_orom *orom;
> } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
> 
> int printf(const char *fmt, ...);
> 
> int isci_pci_probe()
> {
>   int i;
>   struct isci_host *isci_host;
> 
>   for (i = 0, isci_host = v.hosts[i];
>i < 2 && isci_host;
>isci_host = v.hosts[++i]) {
> printf("(%d < %d) == %d\n", i, 2, (i < 2));
>   }
> 
>   return 0;
> }
> 
> int main()
> {
>   isci_pci_probe();
> }
> 
> $ gcc bug.c
> $./a.out
> 0 < 2) == 1
> (1 < 2) == 1
> $ gcc bug.c -O2
> $ ./a.out
> (0 < 2) == 1
> (1 < 2) == 1
> Segmentation fault (core dumped)

Your testcase is invalid:

markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
markus@x4 tmp % ./a.out
(0 < 2) == 1
(1 < 2) == 1
bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host 
*[2]'

As Jakub Jelinek said on IRC, changing the loop to e.g.:

  for (i = 0;
   i < 2 && (isci_host = v.hosts[i]);
   i++) {

fixes the issue.

-- 
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Andi Kleen
Alexei Starovoitov  writes:
>
> disable Value Range Propagation pass:
> -fdisable-tree-vrp1 -fdisable-tree-vrp2
>
> and complete unroll pass:
> -fdisable-tree-cunrolli

Can you file a gcc bug with test case?

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
 wrote:
> On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz  wrote:
>> Hi
>>
>> My story is very simply...
>> I applied the following patch:
>>
>> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> --- a/drivers/scsi/isci/init.c
>> +++ b/drivers/scsi/isci/init.c
>> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
>> struct pci_device_id *id)
>> if (err)
>> goto err_host_alloc;
>>
>> -   for_each_isci_host(i, isci_host, pdev)
>> +   for_each_isci_host(i, isci_host, pdev) {
>> +   pr_err("(%d < %d) == %d\n",\
>> +  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>> scsi_scan_host(to_shost(isci_host));
>> +   }
>>
>> return 0;
>>
>> --
>> 1.8.3.1
>>
>> Then I issued the command 'modprobe isci' on platform with two SCU 
>> controllers (Patsburg D or T chipset)
>> and received the following, very strange, output:
>>
>> (0 < 2) == 1
>> (1 < 2) == 1
>> (2 < 2) == 1
>>
>> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> gcc sees that i < array_size is the same as i < 2 as part of loop condition, 
> so
> it optimizes (i < sci_max_controllers) into constant 1.
> and emits printk like:
>   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>
>> (The kernel was compiled using gcc version 4.8.2.)
>
> it actually looks to be gcc 4.8 bug.
> Can you try gcc 4.7 ?
>
> gcc 4.7 compiles your loop into the following:
> :
>   # i_382 = PHI <0(73), i_73(74)>
>   # isci_host_148 = PHI 
>   printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
>   D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
>   # DEBUG D#6 => isci_host_148
>   # DEBUG ihost s=> ihost
>   scsi_scan_host (D.43295_70);
>   # DEBUG pdev => pdev_17(D)
>   # DEBUG pdev => pdev_17(D)
>   D.43629_353 = dev_get_drvdata (D.42809_20);
>   i_73 = i_382 + 1;
>   # DEBUG i => i_73
>   isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
>   # DEBUG isci_host => isci_host_74
>   # DEBUG isci_host => isci_host_74
>   # DEBUG i => i_73
>   i.9_79 = (unsigned int) i_73;
>   D.42849_65 = i.9_79 <= 1;
>   D.42850_66 = isci_host_74 != 0B;
>   D.42851_67 = D.42850_66 & D.42849_65;
>   if (D.42851_67 != 0)
> goto ;
>   else
> goto ;
>
> which looks correct to me.
>
> while gcc 4.8.2 into:
>   :
>   # i_73 = PHI 
>   # isci_host_274 = PHI 
>   # DEBUG isci_host => isci_host_274
>   # DEBUG i => i_73
>   printk ("\13(%d < %d) == %d\n", i_73, 2, 1);
>   _79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
>   # DEBUG D#6 => isci_host_274
>   # DEBUG ihost => D#6
>   scsi_scan_host (_79);
>   # DEBUG pdev => pdev_26(D)
>   # DEBUG pdev => pdev_26(D)
>   _97 = dev_get_drvdata (_29);
>   i_82 = i_73 + 1;
>   # DEBUG i => i_82
>   isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
>   # DEBUG isci_host => isci_host_83
>   # DEBUG isci_host => isci_host_83
>   # DEBUG i => i_82
>   if (isci_host_83 != 0B)
> goto ;
>   else
> goto ;
>
>   :
>   goto ;
>
> in case of gcc4.8 the i<=1 comparison got optimized out and only
> isci_host !=0 is left,
> which looks incorrect.

It is interesting GCC 4.8 bug,
since it seems to expose issues in two compiler passes.

here is test case:

struct isci_host;
struct isci_orom;

struct isci_pci_info {
  struct isci_host *hosts[2];
  struct isci_orom *orom;
} v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};

int printf(const char *fmt, ...);

int isci_pci_probe()
{
  int i;
  struct isci_host *isci_host;

  for (i = 0, isci_host = v.hosts[i];
   i < 2 && isci_host;
   isci_host = v.hosts[++i]) {
printf("(%d < %d) == %d\n", i, 2, (i < 2));
  }

  return 0;
}

int main()
{
  isci_pci_probe();
}

$ gcc bug.c
$./a.out
0 < 2) == 1
(1 < 2) == 1
$ gcc bug.c -O2
$ ./a.out
(0 < 2) == 1
(1 < 2) == 1
Segmentation fault (core dumped)

workaround:

disable Value Range Propagation pass:
-fdisable-tree-vrp1 -fdisable-tree-vrp2

and complete unroll pass:
-fdisable-tree-cunrolli
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz  wrote:
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> -   for_each_isci_host(i, isci_host, pdev)
> +   for_each_isci_host(i, isci_host, pdev) {
> +   pr_err("(%d < %d) == %d\n",\
> +  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> +   }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU 
> controllers (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?

gcc sees that i < array_size is the same as i < 2 as part of loop condition, so
it optimizes (i < sci_max_controllers) into constant 1.
and emits printk like:
  printk ("\13(%d < %d) == %d\n", i_382, 2, 1);

> (The kernel was compiled using gcc version 4.8.2.)

it actually looks to be gcc 4.8 bug.
Can you try gcc 4.7 ?

gcc 4.7 compiles your loop into the following:
:
  # i_382 = PHI <0(73), i_73(74)>
  # isci_host_148 = PHI 
  printk ("\13(%d < %d) == %d\n", i_382, 2, 1);
  D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
  # DEBUG D#6 => isci_host_148
  # DEBUG ihost s=> ihost
  scsi_scan_host (D.43295_70);
  # DEBUG pdev => pdev_17(D)
  # DEBUG pdev => pdev_17(D)
  D.43629_353 = dev_get_drvdata (D.42809_20);
  i_73 = i_382 + 1;
  # DEBUG i => i_73
  isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
  # DEBUG isci_host => isci_host_74
  # DEBUG isci_host => isci_host_74
  # DEBUG i => i_73
  i.9_79 = (unsigned int) i_73;
  D.42849_65 = i.9_79 <= 1;
  D.42850_66 = isci_host_74 != 0B;
  D.42851_67 = D.42850_66 & D.42849_65;
  if (D.42851_67 != 0)
goto ;
  else
goto ;

which looks correct to me.

while gcc 4.8.2 into:
  :
  # i_73 = PHI 
  # isci_host_274 = PHI 
  # DEBUG isci_host => isci_host_274
  # DEBUG i => i_73
  printk ("\13(%d < %d) == %d\n", i_73, 2, 1);
  _79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
  # DEBUG D#6 => isci_host_274
  # DEBUG ihost => D#6
  scsi_scan_host (_79);
  # DEBUG pdev => pdev_26(D)
  # DEBUG pdev => pdev_26(D)
  _97 = dev_get_drvdata (_29);
  i_82 = i_73 + 1;
  # DEBUG i => i_82
  isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
  # DEBUG isci_host => isci_host_83
  # DEBUG isci_host => isci_host_83
  # DEBUG i => i_82
  if (isci_host_83 != 0B)
goto ;
  else
goto ;

  :
  goto ;

in case of gcc4.8 the i<=1 comparison got optimized out and only
isci_host !=0 is left,
which looks incorrect.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 5:40 PM Sebastian Riemer 
 wrote:
> On 17.01.2014 14:55, Dorau, Lukasz wrote:
> >
> > Some additional information:
> >
> > The loop 'for' in macro ' for_each_isci_host ' defined as
> (drivers/scsi/isci/host.h:313):
> >
> > #define for_each_isci_host(id, ihost, pdev) \
> > for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
> >  id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
> >  ihost = to_pci_info(pdev)->hosts[++id])
> >
> > should be executed only for i = 0 and 1, because:
> > ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
> >
> > but it was executed also for i=2 regardless the above loop's end condition.
> 
> to_pci_info() can return NULL in dev_get_drvdata(). The use of
> ARRAY_SIZE() is inappropriate.
> 
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
> 
> #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> 
> #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> 
> I would say that this was supposed to trigger a build error but it
> didn't and added 1 to the loop end condition.
> 
> Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?
> 

Replacing 'ARRAY_SIZE(to_pci_info(pdev)->hosts)' with 'SCI_MAX_CONTROLLERS' in 
the definition of the ' for_each_isci_host ' macro does not help. I have 
checked it.
The following patch helps:
http://marc.info/?l=linux-scsi=138987823011697=2

Lukasz


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Sebastian Riemer
On 17.01.2014 14:55, Dorau, Lukasz wrote:
> On Friday, January 17, 2014 2:37 PM Dorau, Lukasz  
> wrote:
>>
>> Hi
>>
>> My story is very simply...
>> I applied the following patch:
>>
>> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
>> --- a/drivers/scsi/isci/init.c
>> +++ b/drivers/scsi/isci/init.c
>> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
>> struct
>> pci_device_id *id)
>>  if (err)
>>  goto err_host_alloc;
>>
>> -for_each_isci_host(i, isci_host, pdev)
>> +for_each_isci_host(i, isci_host, pdev) {
>> +pr_err("(%d < %d) == %d\n",\
>> +   i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>>  scsi_scan_host(to_shost(isci_host));
>> +}
>>
>>  return 0;
>>
>> --
>> 1.8.3.1
>>
>> Then I issued the command 'modprobe isci' on platform with two SCU 
>> controllers
>> (Patsburg D or T chipset)
>> and received the following, very strange, output:
>>
>> (0 < 2) == 1
>> (1 < 2) == 1
>> (2 < 2) == 1
>>
>> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>>
>> (The kernel was compiled using gcc version 4.8.2.)
>>
> 
> Some additional information:
> 
> The loop 'for' in macro ' for_each_isci_host ' defined as 
> (drivers/scsi/isci/host.h:313):
> 
> #define for_each_isci_host(id, ihost, pdev) \
> for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
>  id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
>  ihost = to_pci_info(pdev)->hosts[++id])
> 
> should be executed only for i = 0 and 1, because:
> ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2
> 
> but it was executed also for i=2 regardless the above loop's end condition. 

to_pci_info() can return NULL in dev_get_drvdata(). The use of
ARRAY_SIZE() is inappropriate.

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))

#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))

#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))

I would say that this was supposed to trigger a build error but it
didn't and added 1 to the loop end condition.

Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?

Cheers,
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 2:58 PM Richard Weinberger 
 wrote:
> 
> Can you reproduce this using a standalone test?
> I.e:
> #include 
> 
> int main()
> {
> assert(2 < 2 != 1);
> 
> return 0;
> }
> 

No, I can't of course.

Lukasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Richard Weinberger
On Fri, Jan 17, 2014 at 2:37 PM, Dorau, Lukasz  wrote:
> Hi
>
> My story is very simply...
> I applied the following patch:
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *id)
> if (err)
> goto err_host_alloc;
>
> -   for_each_isci_host(i, isci_host, pdev)
> +   for_each_isci_host(i, isci_host, pdev) {
> +   pr_err("(%d < %d) == %d\n",\
> +  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
> scsi_scan_host(to_shost(isci_host));
> +   }
>
> return 0;
>
> --
> 1.8.3.1
>
> Then I issued the command 'modprobe isci' on platform with two SCU 
> controllers (Patsburg D or T chipset)
> and received the following, very strange, output:
>
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
>
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
>
> (The kernel was compiled using gcc version 4.8.2.)
>

Can you reproduce this using a standalone test?
I.e:
#include 

int main()
{
assert(2 < 2 != 1);

return 0;
}

-- 
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 2:37 PM Dorau, Lukasz  
wrote:
> 
> Hi
> 
> My story is very simply...
> I applied the following patch:
> 
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
> struct
> pci_device_id *id)
>   if (err)
>   goto err_host_alloc;
> 
> - for_each_isci_host(i, isci_host, pdev)
> + for_each_isci_host(i, isci_host, pdev) {
> + pr_err("(%d < %d) == %d\n",\
> +i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
>   scsi_scan_host(to_shost(isci_host));
> + }
> 
>   return 0;
> 
> --
> 1.8.3.1
> 
> Then I issued the command 'modprobe isci' on platform with two SCU controllers
> (Patsburg D or T chipset)
> and received the following, very strange, output:
> 
> (0 < 2) == 1
> (1 < 2) == 1
> (2 < 2) == 1
> 
> Can anyone explain why (2 < 2) is true? Is it a gcc bug?
> 
> (The kernel was compiled using gcc version 4.8.2.)
> 

Some additional information:

The loop 'for' in macro ' for_each_isci_host ' defined as 
(drivers/scsi/isci/host.h:313):

#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)->hosts[id]; \
 id < ARRAY_SIZE(to_pci_info(pdev)->hosts) && ihost; \
 ihost = to_pci_info(pdev)->hosts[++id])

should be executed only for i = 0 and 1, because:
ARRAY_SIZE(to_pci_info(pdev)->hosts) = SCI_MAX_CONTROLLERS = 2

but it was executed also for i=2 regardless the above loop's end condition. 

Lukasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Why is (2 < 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
Hi

My story is very simply...
I applied the following patch:

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *id)
if (err)
goto err_host_alloc;
 
-   for_each_isci_host(i, isci_host, pdev)
+   for_each_isci_host(i, isci_host, pdev) {
+   pr_err("(%d < %d) == %d\n",\
+  i, SCI_MAX_CONTROLLERS, (i < SCI_MAX_CONTROLLERS));
scsi_scan_host(to_shost(isci_host));
+   }
 
return 0;
 
-- 
1.8.3.1

Then I issued the command 'modprobe isci' on platform with two SCU controllers 
(Patsburg D or T chipset)
and received the following, very strange, output:

(0 < 2) == 1
(1 < 2) == 1
(2 < 2) == 1

Can anyone explain why (2 < 2) is true? Is it a gcc bug?

(The kernel was compiled using gcc version 4.8.2.)

Lukasz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
Hi

My story is very simply...
I applied the following patch:

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *id)
if (err)
goto err_host_alloc;
 
-   for_each_isci_host(i, isci_host, pdev)
+   for_each_isci_host(i, isci_host, pdev) {
+   pr_err((%d  %d) == %d\n,\
+  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
scsi_scan_host(to_shost(isci_host));
+   }
 
return 0;
 
-- 
1.8.3.1

Then I issued the command 'modprobe isci' on platform with two SCU controllers 
(Patsburg D or T chipset)
and received the following, very strange, output:

(0  2) == 1
(1  2) == 1
(2  2) == 1

Can anyone explain why (2  2) is true? Is it a gcc bug?

(The kernel was compiled using gcc version 4.8.2.)

Lukasz

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 2:37 PM Dorau, Lukasz lukasz.do...@intel.com 
wrote:
 
 Hi
 
 My story is very simply...
 I applied the following patch:
 
 diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
 --- a/drivers/scsi/isci/init.c
 +++ b/drivers/scsi/isci/init.c
 @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
 struct
 pci_device_id *id)
   if (err)
   goto err_host_alloc;
 
 - for_each_isci_host(i, isci_host, pdev)
 + for_each_isci_host(i, isci_host, pdev) {
 + pr_err((%d  %d) == %d\n,\
 +i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
   scsi_scan_host(to_shost(isci_host));
 + }
 
   return 0;
 
 --
 1.8.3.1
 
 Then I issued the command 'modprobe isci' on platform with two SCU controllers
 (Patsburg D or T chipset)
 and received the following, very strange, output:
 
 (0  2) == 1
 (1  2) == 1
 (2  2) == 1
 
 Can anyone explain why (2  2) is true? Is it a gcc bug?
 
 (The kernel was compiled using gcc version 4.8.2.)
 

Some additional information:

The loop 'for' in macro ' for_each_isci_host ' defined as 
(drivers/scsi/isci/host.h:313):

#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
 id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
 ihost = to_pci_info(pdev)-hosts[++id])

should be executed only for i = 0 and 1, because:
ARRAY_SIZE(to_pci_info(pdev)-hosts) = SCI_MAX_CONTROLLERS = 2

but it was executed also for i=2 regardless the above loop's end condition. 

Lukasz

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Richard Weinberger
On Fri, Jan 17, 2014 at 2:37 PM, Dorau, Lukasz lukasz.do...@intel.com wrote:
 Hi

 My story is very simply...
 I applied the following patch:

 diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
 --- a/drivers/scsi/isci/init.c
 +++ b/drivers/scsi/isci/init.c
 @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
 struct pci_device_id *id)
 if (err)
 goto err_host_alloc;

 -   for_each_isci_host(i, isci_host, pdev)
 +   for_each_isci_host(i, isci_host, pdev) {
 +   pr_err((%d  %d) == %d\n,\
 +  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
 scsi_scan_host(to_shost(isci_host));
 +   }

 return 0;

 --
 1.8.3.1

 Then I issued the command 'modprobe isci' on platform with two SCU 
 controllers (Patsburg D or T chipset)
 and received the following, very strange, output:

 (0  2) == 1
 (1  2) == 1
 (2  2) == 1

 Can anyone explain why (2  2) is true? Is it a gcc bug?

 (The kernel was compiled using gcc version 4.8.2.)


Can you reproduce this using a standalone test?
I.e:
#include assert.h

int main()
{
assert(2  2 != 1);

return 0;
}

-- 
Thanks,
//richard
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 2:58 PM Richard Weinberger 
richard.weinber...@gmail.com wrote:
 
 Can you reproduce this using a standalone test?
 I.e:
 #include assert.h
 
 int main()
 {
 assert(2  2 != 1);
 
 return 0;
 }
 

No, I can't of course.

Lukasz

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Sebastian Riemer
On 17.01.2014 14:55, Dorau, Lukasz wrote:
 On Friday, January 17, 2014 2:37 PM Dorau, Lukasz lukasz.do...@intel.com 
 wrote:

 Hi

 My story is very simply...
 I applied the following patch:

 diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
 --- a/drivers/scsi/isci/init.c
 +++ b/drivers/scsi/isci/init.c
 @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
 struct
 pci_device_id *id)
  if (err)
  goto err_host_alloc;

 -for_each_isci_host(i, isci_host, pdev)
 +for_each_isci_host(i, isci_host, pdev) {
 +pr_err((%d  %d) == %d\n,\
 +   i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
  scsi_scan_host(to_shost(isci_host));
 +}

  return 0;

 --
 1.8.3.1

 Then I issued the command 'modprobe isci' on platform with two SCU 
 controllers
 (Patsburg D or T chipset)
 and received the following, very strange, output:

 (0  2) == 1
 (1  2) == 1
 (2  2) == 1

 Can anyone explain why (2  2) is true? Is it a gcc bug?

 (The kernel was compiled using gcc version 4.8.2.)

 
 Some additional information:
 
 The loop 'for' in macro ' for_each_isci_host ' defined as 
 (drivers/scsi/isci/host.h:313):
 
 #define for_each_isci_host(id, ihost, pdev) \
 for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
  id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
  ihost = to_pci_info(pdev)-hosts[++id])
 
 should be executed only for i = 0 and 1, because:
 ARRAY_SIZE(to_pci_info(pdev)-hosts) = SCI_MAX_CONTROLLERS = 2
 
 but it was executed also for i=2 regardless the above loop's end condition. 

to_pci_info() can return NULL in dev_get_drvdata(). The use of
ARRAY_SIZE() is inappropriate.

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))

#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), (a)[0]))

#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))

I would say that this was supposed to trigger a build error but it
didn't and added 1 to the loop end condition.

Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?

Cheers,
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Dorau, Lukasz
On Friday, January 17, 2014 5:40 PM Sebastian Riemer 
sebastian.rie...@profitbricks.com wrote:
 On 17.01.2014 14:55, Dorau, Lukasz wrote:
 
  Some additional information:
 
  The loop 'for' in macro ' for_each_isci_host ' defined as
 (drivers/scsi/isci/host.h:313):
 
  #define for_each_isci_host(id, ihost, pdev) \
  for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
   id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
   ihost = to_pci_info(pdev)-hosts[++id])
 
  should be executed only for i = 0 and 1, because:
  ARRAY_SIZE(to_pci_info(pdev)-hosts) = SCI_MAX_CONTROLLERS = 2
 
  but it was executed also for i=2 regardless the above loop's end condition.
 
 to_pci_info() can return NULL in dev_get_drvdata(). The use of
 ARRAY_SIZE() is inappropriate.
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
 __must_be_array(arr))
 
 #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), (a)[0]))
 
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 
 I would say that this was supposed to trigger a build error but it
 didn't and added 1 to the loop end condition.
 
 Can you test putting SCI_MAX_CONTROLLERS to the loop end condition, please?
 

Replacing 'ARRAY_SIZE(to_pci_info(pdev)-hosts)' with 'SCI_MAX_CONTROLLERS' in 
the definition of the ' for_each_isci_host ' macro does not help. I have 
checked it.
The following patch helps:
http://marc.info/?l=linux-scsim=138987823011697w=2

Lukasz


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com wrote:
 Hi

 My story is very simply...
 I applied the following patch:

 diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
 --- a/drivers/scsi/isci/init.c
 +++ b/drivers/scsi/isci/init.c
 @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
 struct pci_device_id *id)
 if (err)
 goto err_host_alloc;

 -   for_each_isci_host(i, isci_host, pdev)
 +   for_each_isci_host(i, isci_host, pdev) {
 +   pr_err((%d  %d) == %d\n,\
 +  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
 scsi_scan_host(to_shost(isci_host));
 +   }

 return 0;

 --
 1.8.3.1

 Then I issued the command 'modprobe isci' on platform with two SCU 
 controllers (Patsburg D or T chipset)
 and received the following, very strange, output:

 (0  2) == 1
 (1  2) == 1
 (2  2) == 1

 Can anyone explain why (2  2) is true? Is it a gcc bug?

gcc sees that i  array_size is the same as i  2 as part of loop condition, so
it optimizes (i  sci_max_controllers) into constant 1.
and emits printk like:
  printk (\13(%d  %d) == %d\n, i_382, 2, 1);

 (The kernel was compiled using gcc version 4.8.2.)

it actually looks to be gcc 4.8 bug.
Can you try gcc 4.7 ?

gcc 4.7 compiles your loop into the following:
bb 74:
  # i_382 = PHI 0(73), i_73(74)
  # isci_host_148 = PHI isci_host_63(73), isci_host_74(74)
  printk (\13(%d  %d) == %d\n, i_382, 2, 1);
  D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
  # DEBUG D#6 = isci_host_148
  # DEBUG ihost s= ihost
  scsi_scan_host (D.43295_70);
  # DEBUG pdev = pdev_17(D)
  # DEBUG pdev = pdev_17(D)
  D.43629_353 = dev_get_drvdata (D.42809_20);
  i_73 = i_382 + 1;
  # DEBUG i = i_73
  isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
  # DEBUG isci_host = isci_host_74
  # DEBUG isci_host = isci_host_74
  # DEBUG i = i_73
  i.9_79 = (unsigned int) i_73;
  D.42849_65 = i.9_79 = 1;
  D.42850_66 = isci_host_74 != 0B;
  D.42851_67 = D.42850_66  D.42849_65;
  if (D.42851_67 != 0)
goto bb 74;
  else
goto bb 77;

which looks correct to me.

while gcc 4.8.2 into:
  bb 92:
  # i_73 = PHI i_82(93), 0(91)
  # isci_host_274 = PHI isci_host_83(93), isci_host_71(91)
  # DEBUG isci_host = isci_host_274
  # DEBUG i = i_73
  printk (\13(%d  %d) == %d\n, i_73, 2, 1);
  _79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
  # DEBUG D#6 = isci_host_274
  # DEBUG ihost = D#6
  scsi_scan_host (_79);
  # DEBUG pdev = pdev_26(D)
  # DEBUG pdev = pdev_26(D)
  _97 = dev_get_drvdata (_29);
  i_82 = i_73 + 1;
  # DEBUG i = i_82
  isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
  # DEBUG isci_host = isci_host_83
  # DEBUG isci_host = isci_host_83
  # DEBUG i = i_82
  if (isci_host_83 != 0B)
goto bb 93;
  else
goto bb 90;

  bb 93:
  goto bb 92;

in case of gcc4.8 the i=1 comparison got optimized out and only
isci_host !=0 is left,
which looks incorrect.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
alexei.starovoi...@gmail.com wrote:
 On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com wrote:
 Hi

 My story is very simply...
 I applied the following patch:

 diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
 --- a/drivers/scsi/isci/init.c
 +++ b/drivers/scsi/isci/init.c
 @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
 struct pci_device_id *id)
 if (err)
 goto err_host_alloc;

 -   for_each_isci_host(i, isci_host, pdev)
 +   for_each_isci_host(i, isci_host, pdev) {
 +   pr_err((%d  %d) == %d\n,\
 +  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
 scsi_scan_host(to_shost(isci_host));
 +   }

 return 0;

 --
 1.8.3.1

 Then I issued the command 'modprobe isci' on platform with two SCU 
 controllers (Patsburg D or T chipset)
 and received the following, very strange, output:

 (0  2) == 1
 (1  2) == 1
 (2  2) == 1

 Can anyone explain why (2  2) is true? Is it a gcc bug?

 gcc sees that i  array_size is the same as i  2 as part of loop condition, 
 so
 it optimizes (i  sci_max_controllers) into constant 1.
 and emits printk like:
   printk (\13(%d  %d) == %d\n, i_382, 2, 1);

 (The kernel was compiled using gcc version 4.8.2.)

 it actually looks to be gcc 4.8 bug.
 Can you try gcc 4.7 ?

 gcc 4.7 compiles your loop into the following:
 bb 74:
   # i_382 = PHI 0(73), i_73(74)
   # isci_host_148 = PHI isci_host_63(73), isci_host_74(74)
   printk (\13(%d  %d) == %d\n, i_382, 2, 1);
   D.43295_70 = MEM[(struct isci_host *)isci_host_148 + 18632B];
   # DEBUG D#6 = isci_host_148
   # DEBUG ihost s= ihost
   scsi_scan_host (D.43295_70);
   # DEBUG pdev = pdev_17(D)
   # DEBUG pdev = pdev_17(D)
   D.43629_353 = dev_get_drvdata (D.42809_20);
   i_73 = i_382 + 1;
   # DEBUG i = i_73
   isci_host_74 = MEM[(struct isci_pci_info *)D.43629_353].hosts[i_73];
   # DEBUG isci_host = isci_host_74
   # DEBUG isci_host = isci_host_74
   # DEBUG i = i_73
   i.9_79 = (unsigned int) i_73;
   D.42849_65 = i.9_79 = 1;
   D.42850_66 = isci_host_74 != 0B;
   D.42851_67 = D.42850_66  D.42849_65;
   if (D.42851_67 != 0)
 goto bb 74;
   else
 goto bb 77;

 which looks correct to me.

 while gcc 4.8.2 into:
   bb 92:
   # i_73 = PHI i_82(93), 0(91)
   # isci_host_274 = PHI isci_host_83(93), isci_host_71(91)
   # DEBUG isci_host = isci_host_274
   # DEBUG i = i_73
   printk (\13(%d  %d) == %d\n, i_73, 2, 1);
   _79 = MEM[(struct isci_host *)isci_host_274 + 18632B];
   # DEBUG D#6 = isci_host_274
   # DEBUG ihost = D#6
   scsi_scan_host (_79);
   # DEBUG pdev = pdev_26(D)
   # DEBUG pdev = pdev_26(D)
   _97 = dev_get_drvdata (_29);
   i_82 = i_73 + 1;
   # DEBUG i = i_82
   isci_host_83 = MEM[(struct isci_pci_info *)_97].hosts[i_82];
   # DEBUG isci_host = isci_host_83
   # DEBUG isci_host = isci_host_83
   # DEBUG i = i_82
   if (isci_host_83 != 0B)
 goto bb 93;
   else
 goto bb 90;

   bb 93:
   goto bb 92;

 in case of gcc4.8 the i=1 comparison got optimized out and only
 isci_host !=0 is left,
 which looks incorrect.

It is interesting GCC 4.8 bug,
since it seems to expose issues in two compiler passes.

here is test case:

struct isci_host;
struct isci_orom;

struct isci_pci_info {
  struct isci_host *hosts[2];
  struct isci_orom *orom;
} v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};

int printf(const char *fmt, ...);

int isci_pci_probe()
{
  int i;
  struct isci_host *isci_host;

  for (i = 0, isci_host = v.hosts[i];
   i  2  isci_host;
   isci_host = v.hosts[++i]) {
printf((%d  %d) == %d\n, i, 2, (i  2));
  }

  return 0;
}

int main()
{
  isci_pci_probe();
}

$ gcc bug.c
$./a.out
0  2) == 1
(1  2) == 1
$ gcc bug.c -O2
$ ./a.out
(0  2) == 1
(1  2) == 1
Segmentation fault (core dumped)

workaround:

disable Value Range Propagation pass:
-fdisable-tree-vrp1 -fdisable-tree-vrp2

and complete unroll pass:
-fdisable-tree-cunrolli
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Andi Kleen
Alexei Starovoitov alexei.starovoi...@gmail.com writes:

 disable Value Range Propagation pass:
 -fdisable-tree-vrp1 -fdisable-tree-vrp2

 and complete unroll pass:
 -fdisable-tree-cunrolli

Can you file a gcc bug with test case?

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Markus Trippelsdorf
On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
 On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
 alexei.starovoi...@gmail.com wrote:
  On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com 
  wrote:
  Hi
 
  My story is very simply...
  I applied the following patch:
 
  diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
  --- a/drivers/scsi/isci/init.c
  +++ b/drivers/scsi/isci/init.c
  @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, const 
  struct pci_device_id *id)
  if (err)
  goto err_host_alloc;
 
  -   for_each_isci_host(i, isci_host, pdev)
  +   for_each_isci_host(i, isci_host, pdev) {
  +   pr_err((%d  %d) == %d\n,\
  +  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
  scsi_scan_host(to_shost(isci_host));
  +   }
 
  return 0;
 
  --
  1.8.3.1
 
  Then I issued the command 'modprobe isci' on platform with two SCU 
  controllers (Patsburg D or T chipset)
  and received the following, very strange, output:
 
  (0  2) == 1
  (1  2) == 1
  (2  2) == 1
 
  Can anyone explain why (2  2) is true? Is it a gcc bug?
 
  gcc sees that i  array_size is the same as i  2 as part of loop 
  condition, so
  it optimizes (i  sci_max_controllers) into constant 1.
  and emits printk like:
printk (\13(%d  %d) == %d\n, i_382, 2, 1);
 
  (The kernel was compiled using gcc version 4.8.2.)
 
  it actually looks to be gcc 4.8 bug.
  Can you try gcc 4.7 ?
 
 
 It is interesting GCC 4.8 bug,
 since it seems to expose issues in two compiler passes.
 
 here is test case:
 
 struct isci_host;
 struct isci_orom;
 
 struct isci_pci_info {
   struct isci_host *hosts[2];
   struct isci_orom *orom;
 } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};
 
 int printf(const char *fmt, ...);
 
 int isci_pci_probe()
 {
   int i;
   struct isci_host *isci_host;
 
   for (i = 0, isci_host = v.hosts[i];
i  2  isci_host;
isci_host = v.hosts[++i]) {
 printf((%d  %d) == %d\n, i, 2, (i  2));
   }
 
   return 0;
 }
 
 int main()
 {
   isci_pci_probe();
 }
 
 $ gcc bug.c
 $./a.out
 0  2) == 1
 (1  2) == 1
 $ gcc bug.c -O2
 $ ./a.out
 (0  2) == 1
 (1  2) == 1
 Segmentation fault (core dumped)

Your testcase is invalid:

markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
markus@x4 tmp % ./a.out
(0  2) == 1
(1  2) == 1
bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host 
*[2]'

As Jakub Jelinek said on IRC, changing the loop to e.g.:

  for (i = 0;
   i  2  (isci_host = v.hosts[i]);
   i++) {

fixes the issue.

-- 
Markus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Why is (2 2) true? Is it a gcc bug?

2014-01-17 Thread Alexei Starovoitov
On Fri, Jan 17, 2014 at 1:02 PM, Markus Trippelsdorf
mar...@trippelsdorf.de wrote:
 On 2014.01.17 at 11:58 -0800, Alexei Starovoitov wrote:
 On Fri, Jan 17, 2014 at 9:58 AM, Alexei Starovoitov
 alexei.starovoi...@gmail.com wrote:
  On Fri, Jan 17, 2014 at 5:37 AM, Dorau, Lukasz lukasz.do...@intel.com 
  wrote:
  Hi
 
  My story is very simply...
  I applied the following patch:
 
  diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
  --- a/drivers/scsi/isci/init.c
  +++ b/drivers/scsi/isci/init.c
  @@ -698,8 +698,11 @@ static int isci_pci_probe(struct pci_dev *pdev, 
  const struct pci_device_id *id)
  if (err)
  goto err_host_alloc;
 
  -   for_each_isci_host(i, isci_host, pdev)
  +   for_each_isci_host(i, isci_host, pdev) {
  +   pr_err((%d  %d) == %d\n,\
  +  i, SCI_MAX_CONTROLLERS, (i  SCI_MAX_CONTROLLERS));
  scsi_scan_host(to_shost(isci_host));
  +   }
 
  return 0;
 
  --
  1.8.3.1
 
  Then I issued the command 'modprobe isci' on platform with two SCU 
  controllers (Patsburg D or T chipset)
  and received the following, very strange, output:
 
  (0  2) == 1
  (1  2) == 1
  (2  2) == 1
 
  Can anyone explain why (2  2) is true? Is it a gcc bug?
 
  gcc sees that i  array_size is the same as i  2 as part of loop 
  condition, so
  it optimizes (i  sci_max_controllers) into constant 1.
  and emits printk like:
printk (\13(%d  %d) == %d\n, i_382, 2, 1);
 
  (The kernel was compiled using gcc version 4.8.2.)
 
  it actually looks to be gcc 4.8 bug.
  Can you try gcc 4.7 ?
 

 It is interesting GCC 4.8 bug,
 since it seems to expose issues in two compiler passes.

 here is test case:

 struct isci_host;
 struct isci_orom;

 struct isci_pci_info {
   struct isci_host *hosts[2];
   struct isci_orom *orom;
 } v = {{(struct isci_host *)1,(struct isci_host *)1}, 0};

 int printf(const char *fmt, ...);

 int isci_pci_probe()
 {
   int i;
   struct isci_host *isci_host;

   for (i = 0, isci_host = v.hosts[i];
i  2  isci_host;
isci_host = v.hosts[++i]) {
 printf((%d  %d) == %d\n, i, 2, (i  2));
   }

   return 0;
 }

 int main()
 {
   isci_pci_probe();
 }

 $ gcc bug.c
 $./a.out
 0  2) == 1
 (1  2) == 1
 $ gcc bug.c -O2
 $ ./a.out
 (0  2) == 1
 (1  2) == 1
 Segmentation fault (core dumped)

 Your testcase is invalid:

 markus@x4 tmp % clang -fsanitize=undefined -Wall -Wextra -O2 bug.c
 markus@x4 tmp % ./a.out
 (0  2) == 1
 (1  2) == 1
 bug.c:16:20: runtime error: index 2 out of bounds for type 'struct isci_host 
 *[2]'

 As Jakub Jelinek said on IRC, changing the loop to e.g.:

   for (i = 0;
i  2  (isci_host = v.hosts[i]);
i++) {

 fixes the issue.

testcase was reduced from drivers/scsi/isci/host.h written back in
2011 (commit b329aff107)
#define for_each_isci_host(id, ihost, pdev) \
for (id = 0, ihost = to_pci_info(pdev)-hosts[id]; \
 id  ARRAY_SIZE(to_pci_info(pdev)-hosts)  ihost; \
 ihost = to_pci_info(pdev)-hosts[++id])

yes, it does access 3rd element of 2 element array and will read junk.

C standard says the behavior is undefined and comes handy in compiler defense,
but in this case compiler has obviously all the information to make
right decision
instead of misoptimizing the code.
So yes, the loop is erroneous, non-portable, etc, but gcc can be smarter.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/