Re: US-CERT Vulnerability Note VU#162289

[Florian Weimer]

* Brad Roberts:

> Additionally, the linked to notes for GCC are reflective of the original 
> innaccuracies: 
>
> http://www.kb.cert.org/vuls/id/CRDY-7DWKWM
>
> Vendor Statement
> No statement is currently available from the vendor regarding this 
> vulnerability.

Chad, it would be helpful if you could tell the GCC folks what steps
they need to take to publish their view under "Vendor Statement"
(presumably in a link to the GCC web site).

Note to the GCC folks: I've had trouble to get vendor statements
identified as such on the CERT web site before, so if there are some
difficulties, you shouldn't attribute this to the current controversy.

Re: IRA for GCC 4.4

[FX]

I'm willing to try and do some benchmarking of Fortran codes using IRA
(on i686 and x86_64), and report back here with figures and reduced
testcases of eventual slow-downs. What is the current, stable way to
build an IRA compiler and run it? Should I just get the last revision
of the ira branch? When compiling, are there any options
necessary/useful?

Thanks,
FX

-- 
FX Coudert
http://www.homepages.ucl.ac.uk/~uccafco/

Re: IRA for GCC 4.4

[Vladimir Makarov]

FX wrote:

I'm willing to try and do some benchmarking of Fortran codes using IRA
(on i686 and x86_64), and report back here with figures and reduced
testcases of eventual slow-downs. What is the current, stable way to
build an IRA compiler and run it? Should I just get the last revision
of the ira branch? When compiling, are there any options
necessary/useful?

  
The best way to test IRA is to build and use the branch.  It is easy to 
compare the old RA (which is the default on the branch) and IRA (-fira 
option switches IRA on).  I'd recommend to try the following option sets:

 -fira
 -fira -fira-algorithm=CB

I tested IRA mostly on SPEC2000 which, I believe, is a pretty good 
benchmark suite.  It would be interesting how IRA works on other benchmarks.


If you would like to try the mainline, the 2nd version of the patch (I 
sent it a week ago) could be applied but you will need a small fix in 
caller-saves.c.  The difference between in IRA branch and mainline is 
about 3 weeks.  I am going to merge mainline into the branch soon.


I am also going to submit a small patch into the branch on this week 
improving SPECFP2000 performance on ppc about 1% (I did not finished 
patch testing on other ports).  I'll CC it to you.


Vlad

Re: IRA for GCC 4.4

[FX]

>  The best way to test IRA is to build and use the branch.  It is easy to
> compare the old RA (which is the default on the branch) and IRA (-fira
> option switches IRA on).  I'd recommend to try the following option sets:
>   -fira
>   -fira -fira-algorithm=CB

OK, I've done that and I see a 40% to 60% increase in compilation time
for the first (Fortran) testcase I tried, is that expected?

With the compiler from the ira branch on x86_64-linux, here are the
timings reported by "gfortran -c -time -save-temps" with and without
IRA (two timings provided for each set of option, to check
reproducability):

With -O0
# f951 148.97 9.92
# as 3.95 0.18

# f951 137.51 7.05
# as 3.98 0.17

With -O0 -fira
# f951 223.89 10.91
# as 3.67 0.18

# f951 218.98 8.43
# as 3.61 0.19

-O0 -fira -fira-algorithm=CB
# f951 191.32 9.03
# as 3.65 0.15

# f951 190.92 8.96
# as 3.63 0.18


(The testcase is 400k lines of preprocessed Fortran code, 16M is size,
available here:
http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)


FX

-- 
FX Coudert
http://www.homepages.ucl.ac.uk/~uccafco/

Re: IRA for GCC 4.4

[Vladimir Makarov]

FX wrote:

 The best way to test IRA is to build and use the branch.  It is easy to
compare the old RA (which is the default on the branch) and IRA (-fira
option switches IRA on).  I'd recommend to try the following option sets:
  -fira
  -fira -fira-algorithm=CB



OK, I've done that and I see a 40% to 60% increase in compilation time
for the first (Fortran) testcase I tried, is that expected?

  
Yes, that is known problem for -O0.  The old allocator does not use 
global allocator at -O0, IRA is used always even for -O0.  The correct 
comparison would be at -O2.  There are several solution of the problem:
 o We could make only the reload working for -O0.  In this case, the 
time will be the same.
 o We could prevent regional allocation at -O0.  In this case,  the 
slow down would be  20% (I guess).

 o  Use a very fast and simple local allocation.
 o Or just ignore this.

I'd prefer the second  solution.

With the compiler from the ira branch on x86_64-linux, here are the
timings reported by "gfortran -c -time -save-temps" with and without
IRA (two timings provided for each set of option, to check
reproducability):

With -O0
# f951 148.97 9.92
# as 3.95 0.18

# f951 137.51 7.05
# as 3.98 0.17

With -O0 -fira
# f951 223.89 10.91
# as 3.67 0.18

# f951 218.98 8.43
# as 3.61 0.19

-O0 -fira -fira-algorithm=CB
# f951 191.32 9.03
# as 3.65 0.15

# f951 190.92 8.96
# as 3.63 0.18


(The testcase is 400k lines of preprocessed Fortran code, 16M is size,
available here:
http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)

  

Thanks, I'll check it.

Re: IRA for GCC 4.4

[FX]

>  Yes, that is known problem for -O0.  The old allocator does not use global
> allocator at -O0, IRA is used always even for -O0.  The correct comparison
> would be at -O2.

Well, I guess it depends on what you understand by "correct". I guess
to users, the correct comparison is whatever they are using most
often. So, even though I'm not technically competent to help you
deciding which choice is the better, I might still want to say that:

>   o Or just ignore this.

seems certainly a bad option to me.

I'll run some more Fortran tests (both compilation timings and runtime
performance with various options) during the next few days, and I'll
post a summary to the list later on.

Thanks for your quick reply,
FX

-- 
FX Coudert
http://www.homepages.ucl.ac.uk/~uccafco/

Re: IRA for GCC 4.4

[Paolo Bonzini]

(The testcase is 400k lines of preprocessed Fortran code, 16M is size,
available here:
http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)

  

Thanks, I'll check it.


Vlad, I think you should also try to understand what does trunk do with 
 global (and without local allocation) at -O0.  That will give a 
measure of the benefit from Peter's patches for conflict graph building.


Another thing to evaluate is the impact of changing gimplify.c so that 
it always follows the "if (optimize)" paths.  The differences are there 
exactly because we don't run global register allocation at -O0, and they 
create more pseudos.


Paolo

Re: US-CERT Vulnerability Note VU#162289

[Neil Booth]

David Miller wrote:-

> From: Joe Buck <[EMAIL PROTECTED]>
> Date: Wed, 23 Apr 2008 08:24:44 -0700
> 
> > If CERT is to maintain its reputation, it needs to do better.  The warning
> > is misdirected in any case; given the very large number of compilers that
> > these coding practices cause trouble for, you need to focus on the bad
> > coding practices, not on unfair demonization of new GCC releases.
> 
> In my opinion CERT's advisory has been nothing but an unfair FUD
> campaign on compilers, and GCC specifically, and has seriously
> devalued CERT's advisories, in general, which were already of low
> value to begin with.
> 
> It looks similar to a news article run by a newspaper that is losing
> money and has no real news to write about, but yet they have to write
> about something.
> 
> The worst part of this fiasco is that GCCs reputation has been
> unfairly harmed in one way or another, and there is nothing CERT can
> do to rectify the damage they've caused.

I'm appalled that the original desciption hasn't been corrected.  The
text reads:

  Some C compilers optimize away pointer arithmetic overflow tests that
  depend on undefined behavior without providing a diagnostic (a warning).
  Applications containing these tests may be vulnerable to buffer
  overflows if compiled with these compilers.

  I. Description
  In the C language, given the types:

char *buf;
int len;

  some C compilers will assume that buf+len >= buf. 

which is an entirely bogus description of the problem.  That this
incorrect description of the state of affairs has been left to
stand only shows that CERT, and those responsible for this advisory,
have completely failed to understand what the real issue is here.
Further, the fact that the "advisory" stands in this erroneous
form, despite it having been pointed out to them many times over
the past weeks on this form at least, seriously undermines their
credibility in the eyes of any informed observer.

At a minimum the wording should be something more like:

  In the C language, given an object OBJ and a pointer BUF into OBJ,

char *buf;
int len;

  the C standard requires that the result of

buf + len

  must point within, or one byte beyond, the object BUF.  A program
  that does not satisfy this constraint is erroneous, and many
  compilers take advantage of this constraint to optimize code more
  effectively.  Unforunately much existing code is not well written
  and sometimes erroneous in this regard, and hence may not behave
  as originally intended when compiled with optimizations enabled.

Neil.

Question about compilation details

[eduardo]

   Hi.
   I'm using the arm-elf-gcc compiler to compile some files to a ARM9 
(Freescale i.MX27, ARM926EJ-S core). What I would like to know is how can I 
visualize/change the address specifications made by the linker to the output 
file? I heard gcc can generate a file (.lst) that contains such information, 
but I also don't know how to create this file.
  Thanks in advance.
   Best regards,

   Eduardo

Re: US-CERT Vulnerability Note VU#162289

[Robert C. Seacord]

Neil,

I'm not sure I understand what you mean by the following:

A program that does not satisfy this constraint is erroneous, and many compilers take advantage of this constraint to optimize code more effectively. 

Just because a program contains undefined behavior, does not mean that it 
erroneous. It simply gives the compiler latitude with how to handle the 
undefined behavior, while still conforming.

One possibility is that GCC could handle these constructs in a consistent manner.  That is, GCC clearly implements modwrap semantics. Given this, I think the behavior exhibited in this case is inconsistent.  If, on the other hand, GCC implemented saturation semantics, it would make perfect sense to optimize out this check.  


I'll review your comments below and consider additional changes to the text of 
the vul note.  We would really like to see is a diagnostic for this condition, 
ideally enabled by -Wall.  Once a version of the compiler is available that 
provides such a diagnostic is available, I will recommend we change the US-CERT 
Addendum to recommend that developers upgrade to that version of the compiler, 
and compile using -Wall or whatever the appropriate flag turns out to be.

I am getting tired with the personal/organizational attacks.  If you expect a 
response, please keep your comments professional.

rCs




David Miller wrote:-

  

From: Joe Buck <[EMAIL PROTECTED]>
Date: Wed, 23 Apr 2008 08:24:44 -0700



If CERT is to maintain its reputation, it needs to do better.  The warning
is misdirected in any case; given the very large number of compilers that
these coding practices cause trouble for, you need to focus on the bad
coding practices, not on unfair demonization of new GCC releases.
  

In my opinion CERT's advisory has been nothing but an unfair FUD
campaign on compilers, and GCC specifically, and has seriously
devalued CERT's advisories, in general, which were already of low
value to begin with.

It looks similar to a news article run by a newspaper that is losing
money and has no real news to write about, but yet they have to write
about something.

The worst part of this fiasco is that GCCs reputation has been
unfairly harmed in one way or another, and there is nothing CERT can
do to rectify the damage they've caused.



I'm appalled that the original desciption hasn't been corrected.  The
text reads:

  Some C compilers optimize away pointer arithmetic overflow tests that
  depend on undefined behavior without providing a diagnostic (a warning).
  Applications containing these tests may be vulnerable to buffer
  overflows if compiled with these compilers.

  I. Description
  In the C language, given the types:

char *buf;
int len;

  some C compilers will assume that buf+len >= buf. 


which is an entirely bogus description of the problem.  That this
incorrect description of the state of affairs has been left to
stand only shows that CERT, and those responsible for this advisory,
have completely failed to understand what the real issue is here.
Further, the fact that the "advisory" stands in this erroneous
form, despite it having been pointed out to them many times over
the past weeks on this form at least, seriously undermines their
credibility in the eyes of any informed observer.

At a minimum the wording should be something more like:

  In the C language, given an object OBJ and a pointer BUF into OBJ,

char *buf;
int len;

  the C standard requires that the result of

buf + len

  must point within, or one byte beyond, the object BUF.  A program
  that does not satisfy this constraint is erroneous, and many
  compilers take advantage of this constraint to optimize code more
  effectively.  Unforunately much existing code is not well written
  and sometimes erroneous in this regard, and hence may not behave
  as originally intended when compiled with optimizations enabled.

Neil.
  

Re: US-CERT Vulnerability Note VU#162289

[Andreas Schwab]

Neil Booth <[EMAIL PROTECTED]> writes:

> At a minimum the wording should be something more like:
>
>   In the C language, given an object OBJ and a pointer BUF into OBJ,
>
> char *buf;
> int len;
>
>   the C standard requires that the result of
>
>   buf + len
>
>   must point within, or one byte beyond, the object BUF.

ITYM OBJ here-^^^

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: US-CERT Vulnerability Note VU#162289

[Andrew Haley]

Robert C. Seacord wrote:
> Neil,
> 
> I'm not sure I understand what you mean by the following:
> 
>> A program that does not satisfy this constraint is erroneous, and many
>> compilers take advantage of this constraint to optimize code more
>> effectively. 
> Just because a program contains undefined behavior, does not mean that
> it erroneous.

This is the crux of our disagreement.  To me, and I imagine almost
everyone else on the gcc list, any program that contains undefined 
behaviour is *by definition* erroneous.

It is erroneous because there is no way to determine what the program
should do.  The program is, quite literally, meaningless.

Certainly, a compiler writer can extend the language to give a compiler-
specific definition to that behaviour, in which case it's no longer
undefined.  But that is not true in this particular case.

> One possibility is that GCC could handle these constructs in a
> consistent manner.  That is, GCC clearly implements modwrap semantics.
> Given this, I think the behavior exhibited in this case is inconsistent.
> If, on the other hand, GCC implemented saturation semantics, it
> would make perfect sense to optimize out this check.

gcc implements ISO C semantics, with some extensions.  We could
extend the language in the way you suggest, but it would be very
difficult formally to specify such an extension.  I don't think
it's something we should do.

Andrew.

Re: Official GCC git repository

[Christopher Faylor]

On Wed, Apr 23, 2008 at 05:14:42PM -0400, Christopher Faylor wrote:
>On Wed, Apr 23, 2008 at 08:09:57PM +0200, Samuel Tardieu wrote:
>>> "Christopher" == Christopher Faylor <[EMAIL PROTECTED]> writes:
>>
>>Christopher> After consultation with Dan, I have set things up on
>>Christopher> gcc.gnu.org so that the git repository is updated every
>>Christopher> time an email message is received from the gcc-cvs
>>Christopher> mailing list.
>>
>>Christopher> We'll be monitoring the system to see if there is a load
>>Christopher> hit.  If so, we'll probably drop back to Dan's original
>>Christopher> method.
>>
>>It looks like the GIT repository hasn't been synced since yesterday.
>>
>
>Yes:
>
>Apr 23 20:41:28 sourceware error: invalid object 
>6c380017b3c68b8efc917ae3447e62b0fd76b009
>Apr 23 20:41:28 sourceware fatal: git-write-tree: error building trees; the 
>index is unmerged?
>Apr 23 20:41:28 sourceware write-tree: command returned error: 128
>
>What fun!

In case it isn't obvious, I don't know how to fix this.

cgf

Re: Official GCC git repository

[Samuel Tardieu]

On 24/04, Christopher Faylor wrote:

| In case it isn't obvious, I don't know how to fix this.

Well, I guess it fixed itself then since it's now working again :)

Re: US-CERT Vulnerability Note VU#162289

[Mark Mitchell]

Robert C. Seacord wrote:

Just because a program contains undefined behavior, does not mean that 
it erroneous. It simply gives the compiler latitude with how to handle 
the undefined behavior, while still conforming.


Actually, erroneous and undefined are approximately synonymous.

There are various categories of behavior not mandated by the standard. 
The standard identifies:


* implementation-defined behavior

unspecified behavior where each implementation documents how the choice 
is made


* unspecified behavior

behavior where this International Standard provides two or more 
possibilities and imposes no further requirements on which is chosen in 
any instance


* undefined behavior

behavior, upon use of a nonportable or erroneous program construct or of 
erroneous data, for which this International Standard imposes no 
requirements


And:


Addition or subtraction of a pointer into, or just beyond, an array object and 
an integer type produces a result that does not point into, or just beyond, the 
same array object (6.5.6).


is undefined behavior.

This is the same class as using the value of an uninitialized variable, 
dividing by zero, or any other erroneous construct that cannot be 
diagnosed at compile-time.  From the point of view of the standard, your 
example of a "secure" program is no more portable than one that reads 
random uninitialized memory and depends on the result.


That said, the GCC developers have already implemented a warning option 
to warn about the case where optimizations are made on this basis.  I do 
not know if it is in -Wall or not.  I doubt it, because it would 
probably create far too many false positives.


Declaring that a compiler is "vulnerable" because it performs a valid 
optimization performed by most of the leading compilers is somewhat odd. 
 Declaring it vulnerable while not declaring those others to be 
vulnerable is unfair.  However, I do appreciate that issues in GCC is 
probably more important than issues in other compilers, given GCC's 
overwhelming popularity and the increasing degree to which vitally 
important software is built with GCC.


--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713

Re: Official GCC git repository

[Daniel Berlin]

It fixed itself. :)

On Thu, Apr 24, 2008 at 12:09 PM, Christopher Faylor
<[EMAIL PROTECTED]> wrote:
>
> On Wed, Apr 23, 2008 at 05:14:42PM -0400, Christopher Faylor wrote:
>  >On Wed, Apr 23, 2008 at 08:09:57PM +0200, Samuel Tardieu wrote:
>  >>> "Christopher" == Christopher Faylor <[EMAIL PROTECTED]> writes:
>  >>
>  >>Christopher> After consultation with Dan, I have set things up on
>  >>Christopher> gcc.gnu.org so that the git repository is updated every
>  >>Christopher> time an email message is received from the gcc-cvs
>  >>Christopher> mailing list.
>  >>
>  >>Christopher> We'll be monitoring the system to see if there is a load
>  >>Christopher> hit.  If so, we'll probably drop back to Dan's original
>  >>Christopher> method.
>  >>
>  >>It looks like the GIT repository hasn't been synced since yesterday.
>  >>
>  >
>  >Yes:
>  >
>  >Apr 23 20:41:28 sourceware error: invalid object 
> 6c380017b3c68b8efc917ae3447e62b0fd76b009
>  >Apr 23 20:41:28 sourceware fatal: git-write-tree: error building trees; the 
> index is unmerged?
>  >Apr 23 20:41:28 sourceware write-tree: command returned error: 128
>  >
>  >What fun!
>
>  In case it isn't obvious, I don't know how to fix this.
>
>  cgf
>

Re: IRA for GCC 4.4

[Vladimir Makarov]

FX wrote:

 The best way to test IRA is to build and use the branch.  It is easy to
compare the old RA (which is the default on the branch) and IRA (-fira
option switches IRA on).  I'd recommend to try the following option sets:
  -fira
  -fira -fira-algorithm=CB



OK, I've done that and I see a 40% to 60% increase in compilation time
for the first (Fortran) testcase I tried, is that expected?

With the compiler from the ira branch on x86_64-linux, here are the
timings reported by "gfortran -c -time -save-temps" with and without
IRA (two timings provided for each set of option, to check
reproducability):

With -O0
# f951 148.97 9.92
# as 3.95 0.18

# f951 137.51 7.05
# as 3.98 0.17

With -O0 -fira
# f951 223.89 10.91
# as 3.67 0.18

# f951 218.98 8.43
# as 3.61 0.19

-O0 -fira -fira-algorithm=CB
# f951 191.32 9.03
# as 3.65 0.15

# f951 190.92 8.96
# as 3.63 0.18


(The testcase is 400k lines of preprocessed Fortran code, 16M is size,
available here:
http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)


  
Thanks for extremely interesting test (with more 3500 big functions in 
one file).  It seems that the difference in cpu time is not so big but 
the wall time difference is really big.  The reason for this is memory 
consumption.  Even with the old allocator gcc needs more 1GB for -O0.  
With IRA it needs more 2GB memory.  The same problem would be if 
global.c were used for -O0.


The following patch brings the same memory consumption for -fira -O0 as 
for the old allocator.  The patch decreases the difference in wall time 
to 13-20% (I can see 10% discrepancy in wall time even for the old 
allocator on my 2GB Core2 machine.  I found it depends on did I start 
firefox on my machine or not).  I'll commit the patch after thorough 
testing.


It is a temporary solution,  I think I'll have better solution in 2-3 
weeks bringing the wall time really close to the old allocator.





Index: cfgloopanal.c
===
--- cfgloopanal.c	(revision 134601)
+++ cfgloopanal.c	(working copy)
@@ -390,8 +390,8 @@ estimate_reg_pressure_cost (unsigned n_n
one.  */
 cost = target_spill_cost * n_new;
 
-  if (flag_ira && (flag_ira_algorithm == IRA_ALGORITHM_REGIONAL
-		   || flag_ira_algorithm == IRA_ALGORITHM_MIXED)
+  if (optimize && flag_ira && (flag_ira_algorithm == IRA_ALGORITHM_REGIONAL
+			   || flag_ira_algorithm == IRA_ALGORITHM_MIXED)
   && number_of_loops () <= (unsigned) IRA_MAX_LOOPS_NUM)
 /* IRA regional allocation deals with high register pressure
better.  So decrease the cost (to do more accurate the cost
Index: caller-save.c
===
--- caller-save.c	(revision 134601)
+++ caller-save.c	(working copy)
@@ -457,7 +457,7 @@ setup_save_areas (void)
 	unsigned int regno = reg_renumber[i];
 	unsigned int endregno
 	  = end_hard_regno (GET_MODE (regno_reg_rtx[i]), regno);
-	if (flag_ira && flag_ira_ipra)
+	if (flag_ira && optimize && flag_ira_ipra)
 	  {
 	HARD_REG_SET clobbered_regs;
 	
@@ -472,7 +472,7 @@ setup_save_areas (void)
 	  SET_HARD_REG_BIT (hard_regs_used, r);
   }
 
-  if (flag_ira && flag_ira_share_save_slots)
+  if (flag_ira && optimize && flag_ira_share_save_slots)
 {
   rtx insn, slot;
   struct insn_chain *chain, *next;
@@ -857,7 +857,7 @@ calculate_local_save_info (void)
 		  
 		  /* Remember live_throughout can contain spilled
 		 registers when IRA is used.  */
-		  if (flag_ira && r < 0)
+		  if (flag_ira && optimize && r < 0)
 		continue;
 		  gcc_assert (r >= 0);
 		  nregs = hard_regno_nregs[r][PSEUDO_REGNO_MODE (regno)];
@@ -1203,7 +1203,7 @@ save_call_clobbered_regs (void)
   struct insn_chain *chain, *next;
   enum machine_mode save_mode[FIRST_PSEUDO_REGISTER];
 
-  if (flag_ira && flag_ira_move_spills)
+  if (flag_ira && optimize && flag_ira_move_spills)
 {
   /* Do global analysis for better placement of spill code. */
   alloc_aux_for_blocks (sizeof (struct bb_info));
@@ -1248,7 +1248,7 @@ save_call_clobbered_regs (void)
 
 		regno += insert_restore (chain, 1, regno, MOVE_MAX_WORDS,
 	 save_mode);
-		if (flag_ira && flag_ira_move_spills)
+		if (flag_ira && optimize && flag_ira_move_spills)
 		  {
 			gcc_assert (before == regno);
 			save_mode[before] = VOIDmode;
@@ -1291,7 +1291,7 @@ save_call_clobbered_regs (void)
 
 		  /* Remember live_throughout can contain spilled
 		 registers when IRA is used.  */
-		  if (flag_ira && r < 0)
+		  if (flag_ira && optimize && r < 0)
 		continue;
 		  gcc_assert (r >= 0);
 		  nregs = hard_regno_nregs[r][PSEUDO_REGNO_MODE (regno)];
@@ -1343,7 +1343,7 @@ save_call_clobbered_regs (void)
 	 remain saved.  If the last insn in the block is a JUMP_INSN, put
 	 the restore before the insn, otherwise, put it after the insn.  */
 
-	  if (flag_ira && flag_ira_move_spills)
+	  if (flag_ira && optimize &&

Use of option -fprofile-arcs is not compatible with -fprofile-use

[Edmar Wienskoski-RA8797]

The test case gcc.misc/bprob-1.c is compiled with fprofile-arcs / 
fprofile-use.


The option fprofile-arcs does not enable value profiling.

At the second stage compilation, the option fprofile-use enables value 
profiling. Within tree_find_values_to_profile, if one of the value 
optimizations algorithms sees an optimization opportunity, it will push 
an histogram on stack. Later, compute_value_histograms will call 
get_coverage_counts to load this histogram, but none where generated.


A warning is issued which means a FAIL under dejagnu.

I found this problem with bprob-1.c while debugging a new value profile 
optimization. But it can be reproduced in any target, with non-modified 
gcc, at any optimization level, using one of the value profile test 
cases and compiler options fprofile-arcs / fprofile-use (same used with 
bprob-1.c).


Here is an example using gcc.dg/tree-prof/val-prof-1.c:
./gcc-trunk-reference/install_e600/bin/gcc -g -fprofile-arcs 
val-prof-1.c -o val-prof-1.x1

./val-prof-1.x1
./gcc-trunk-reference/install_e600/bin/gcc -g -fprofile-use val-prof-1.c 
-o val-prof-1.x2

val-prof-1.c: In function ‘main’:
val-prof-1.c:17: warning: no coverage for function ‘main’ found

IMHO there are 3 ways to go with this:
1 - Require user behavior change (create new option -fprofile-arcs-use 
to match -fprofile-arcs, mismatch of options is bad user behavior)
2 - Record on the .gcda file how the first stage were done 
(fprofile-arcs / fprofile-generate, etc) and use it to disable other 
optimizations under fprofile-use (Does this already exists ?, I am not 
familiar with the .gcda layout)

3 - Let get_coverage_counts ignore inconsistencies when loading data.

Helps / comments are appreciated.

Edmar

Re: IRA for GCC 4.4

[Joe Buck]

On Thu, Apr 24, 2008 at 10:42:49AM -0400, Vladimir Makarov wrote:
> FX wrote:
> >> The best way to test IRA is to build and use the branch.  It is easy to
> >>compare the old RA (which is the default on the branch) and IRA (-fira
> >>option switches IRA on).  I'd recommend to try the following option sets:
> >>  -fira
> >>  -fira -fira-algorithm=CB
> >>
> >
> >OK, I've done that and I see a 40% to 60% increase in compilation time
> >for the first (Fortran) testcase I tried, is that expected?
> >
> >  
> Yes, that is known problem for -O0.  The old allocator does not use 
> global allocator at -O0, IRA is used always even for -O0.  The correct 
> comparison would be at -O2.  There are several solution of the problem:
>  o We could make only the reload working for -O0.  In this case, the 
> time will be the same.
>  o We could prevent regional allocation at -O0.  In this case,  the 
> slow down would be  20% (I guess).
>  o  Use a very fast and simple local allocation.
>  o Or just ignore this.
> 
> I'd prefer the second  solution.

Ignoring it isn't an option; developers who spend most of their
time in the compile/debug/test/fix loop would see a 40-60% decrease at -O0
as a very severe regression.  Even 20% would get a lot of complaints.

Re: IRA for GCC 4.4

[Vladimir Makarov]

Joe Buck wrote:

On Thu, Apr 24, 2008 at 10:42:49AM -0400, Vladimir Makarov wrote:
  

FX wrote:


The best way to test IRA is to build and use the branch.  It is easy to
compare the old RA (which is the default on the branch) and IRA (-fira
option switches IRA on).  I'd recommend to try the following option sets:
 -fira
 -fira -fira-algorithm=CB
   


OK, I've done that and I see a 40% to 60% increase in compilation time
for the first (Fortran) testcase I tried, is that expected?

 
  
Yes, that is known problem for -O0.  The old allocator does not use 
global allocator at -O0, IRA is used always even for -O0.  The correct 
comparison would be at -O2.  There are several solution of the problem:
 o We could make only the reload working for -O0.  In this case, the 
time will be the same.
 o We could prevent regional allocation at -O0.  In this case,  the 
slow down would be  20% (I guess).

 o  Use a very fast and simple local allocation.
 o Or just ignore this.

I'd prefer the second  solution.



Ignoring it isn't an option; developers who spend most of their
time in the compile/debug/test/fix loop would see a 40-60% decrease at -O0
as a very severe regression.  Even 20% would get a lot of complaints.

  

Yes, that was probably a stupid remark from me.  Sorry.

I already made a temporary solution bringing it to 15% for this extreme 
test which requires more 1GB for -O0 and more 2Gb for -O2 (all with the 
old register allocator).  As I wrote I'll have a better solution in 3 weeks.

Re: US-CERT Vulnerability Note VU#162289

[Mark Mitchell]

Robert C. Seacord wrote:

The following article encapsulates my understanding of undefined 
behavior based on these discussions:


MSC15-A. Do not depend on undefined behavior 



I think that looks like a fine article, at first glance.  But, of 
course, the behavior is undefined, so, as your article says, people 
shouldn't use it.


That said, the GCC developers have already implemented a warning 
option to warn about the case where optimizations are made on this 
basis.  I do not know if it is in -Wall or not.  I doubt it, because 
it would probably create far too many false positives.


I believe that if you use the option -Wstrict-overflow=5 the development 
version of the compiler will warn about this test case.  This option was 
implemented in response to your report, and will be in GCC 4.4.  Users 
who want to can of course download the development source code for GCC 
and build it today, even before it is released, to check their code.  (I 
wouldn't recommend using the development version to build things, but 
people could use it to get the warning, if they want to do that.)


Declaring it vulnerable while not declaring those others to be 
vulnerable is unfair.  


We are still evaluating other compilers.  If we find that they exhibit 
the same behavior, we will list them as vulnerable as well.


When can we expect that you will have completed that investigation?  I 
can appreciate the desire for an independent investigation, but given 
the data we have already provided, it should be a pretty simple matter 
to verify this.


Thanks,

--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713

Security vulernarability or security feature?

[Ralph Loader]

Dear "Cert",

I originally raised this with you privately, but you are slow to
respond, so I am raising this again, more concisely, and CC'd to a less
private forum.

(a) Arithmetic overflows have historically been a significant source of
security vulnerabilities.

(b) Recent versions of gcc (along with other compilers) contain an
optimisation that can *REMOVE* arithmetic overflows.

Why is Cert advising people to avoid an optimisation that can ---
realistically, although probably rarely --- remove security
vulnerabilities?

[I also note that the example you claim is a "length check" in your
advisory, is nothing of the sort.  It is an oddly written test of the
absolute position of a pointer.  I don't actually see how the
optimisation in question could remove a check on the length of
something.  And even more, I don't see how such a hypothetical length
check could not also avoid being broken by other 101 other things,
such as variations in OS memory layout, which may vary even between
successive runs of identical binarys]

Ralph.

Re: Security vulernarability or security feature?

[Robert C. Seacord]

Ralph,

Comments below.


(a) Arithmetic overflows have historically been a significant source of
security vulnerabilities.
  

agreed.

(b) Recent versions of gcc (along with other compilers) contain an
optimisation that can *REMOVE* arithmetic overflows.
  
I am very interested in seeing how this optimization can remove 
arithmetic overflows.


If you can send me an example of source code and instructions on how to 
build, I would certainly be happy to promote this feature of gcc on our 
secure coding web site.



Why is Cert advising people to avoid an optimisation that can ---
realistically, although probably rarely --- remove security
vulnerabilities?
  
If you are referring to VU#694123, this refers to an optimization that 
removes checks pointer arithmetic wrapping.  The optimization doesn't 
actually eliminate the wrapping behavior; this still occurs.  It does, 
however, eliminate certain kinds of checks (that depend upon undefined 
behavior).


Thanks,
rCs

Re: IRA for GCC 4.4

[Peter Bergner]

On Thu, 2008-04-24 at 16:51 +0200, Paolo Bonzini wrote:
> >> (The testcase is 400k lines of preprocessed Fortran code, 16M is size,
> >> available here:
> >> http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)
> >>
> >>   
> > Thanks, I'll check it.
> 
> Vlad, I think you should also try to understand what does trunk do with 
>   global (and without local allocation) at -O0.  That will give a 
> measure of the benefit from Peter's patches for conflict graph building.

I took a patch from Ken/Steven that disabled local_alloc and instead runs
global_alloc() at -O0 and summing up all of the bit matrix allocation
info we emit into the *.greg output, the new conflict builder saves a lot
of space compared to the old square bit matrix (almost 20x less space).
Here's the accumulated data for the test case above:

compressed upper triangular:  431210251 bits,   53902848 bytes
upper triangular:4264666581 bits,  533084851 bytes
square:  8531372796 bits, 1066423618 bytes

Peter

Re: Security vulernarability or security feature?

[Ralph Loader]

> I am very interested in seeing how this optimization can remove 
> arithmetic overflows.

int foo (char * buf, int n)
{
// buf+n may overflow of the programmer incorrectly passes
// a large value of n.  But recent versions of gcc optimise
// to 'n < 100', removing the overflow.
return buf + n < buf + 100;
}

Compiled on i386, gcc-4.3.0 with -O2 gives:

foo:
xorl%eax, %eax
cmpl$99, 8(%esp)
setle   %al
ret

E.g., calling foo with:

#include 
int main()
{
char buf[100];
printf ("%d\n", foo (buf, 15));
return 0;
}

on my PC (where the stack is just below the 3Gig position).


> > Why is Cert advising people to avoid an optimisation that can ---
> > realistically, although probably rarely --- remove security
> > vulnerabilities?
> >   
> If you are referring to VU#694123, this refers to an optimization

I'm talking about 162289.

Ralph.

> that removes checks pointer arithmetic wrapping.  The optimization
> doesn't actually eliminate the wrapping behavior; this still occurs.
> It does, however, eliminate certain kinds of checks (that depend upon
> undefined behavior).
> 
> Thanks,
> rCs

Re: Security vulernarability or security feature?

[Ralph Loader]

Robert,

You have failed to answer my original question, and I think have failed
to understand the point of the example.

The example shows that what you are claiming is a vulnerability in
162289 is in fact a security feature.

>   that removes checks pointer arithmetic wrapping.

Just to be 100% clear, the optimisation I gave you an example of, and
which you think is "pretty cool" is precisely the same optimisation you
are complaining about in 162289, specifically, a pointer value may be
canceled from both sides of a comparison.

This is slightly confused by the fact that in the 162289 example, two
optimisations take place [gcc gurus please correct if I am wrong]:

(a) a pointer value is canceled from both sides of a comparison,
changing (buf+len  The optimization
> doesn't actually eliminate the wrapping behavior; this still occurs.
> It does, however, eliminate certain kinds of checks (that depend upon
> undefined behavior).
> 
> Thanks,
> rCs
> 

Re: IRA for GCC 4.4

[Peter Bergner]

On Thu, 2008-04-24 at 16:33 -0500, Peter Bergner wrote:
> On Thu, 2008-04-24 at 16:51 +0200, Paolo Bonzini wrote:
> > >> (The testcase is 400k lines of preprocessed Fortran code, 16M is size,
> > >> available here:
> > >> http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)
> > >>
> > >>   
> > > Thanks, I'll check it.
> > 
> > Vlad, I think you should also try to understand what does trunk do with 
> >   global (and without local allocation) at -O0.  That will give a 
> > measure of the benefit from Peter's patches for conflict graph building.
> 
> I took a patch from Ken/Steven that disabled local_alloc and instead runs
> global_alloc() at -O0 and summing up all of the bit matrix allocation
> info we emit into the *.greg output, the new conflict builder saves a lot
> of space compared to the old square bit matrix (almost 20x less space).
> Here's the accumulated data for the test case above:
> 
> compressed upper triangular:  431210251 bits,   53902848 bytes
> upper triangular:4264666581 bits,  533084851 bytes
> square:  8531372796 bits, 1066423618 bytes

The SPEC2000 numbers look even better (29x less space):

compressed upper triangular:  281657797 bits,   35212532 bytes
upper triangular 4094809686 bits,  511856604 bytes
square:  8191641644 bits, 1023962188 bytes

Peter

Re: dg-skip-if on powerpc when multiple cpu cflags specified

[Janis Johnson]

On Wed, 2008-04-23 at 10:56 -0500, Joel Sherrill wrote:
> Hi,
> 
> I am returning to this issue and it is more
> pressing testing powerpc on 4.3.0 and the trunk.
> powerpc-rtems has gone from a relatively small
> percentage of failures to >8300 and this warning
> shows up a lot (5120334 times)!
> 
>  Warning: /home/joel/work-gnat/svn/b-gcc1-powerpc/rtems_gcc_main.o uses 
> hard float, 
> /home/joel/work-gnat/svn/b-gcc1-powerpc/gcc/testsuite/gcc/20010124-1.x0 
> uses soft float
> 
> $ grep "hard float" ./gcc/testsuite/gcc/gcc.log | wc -l
> 5120334
> 
> The target is psim/603e. 
> rtems_gcc_main.o is the configuration file and RTEMS
> initialization task setup for the gcc tests. 
> This file and the RTEMS library for were compiled with
> hard float.
> 
> On this target, we really need to skip these tests to
> get rid of all this noise.  I can't even begin to tell what
> else is broken.
> 
> I had the impression you weren't happy with the code
> you proposed and didn't sweep it in.   Is it an acceptable
> solution?  Or do you all have another idea?

I don't think it's workable, but I understand that the
problem does require a solution.  Is this a fair statement
of the problem?

  Multilib options can include -mcpu= or -march=.

  Many target-specific tests use dg-options to specify use
  of -mcpu= or -march= for that test.

  Multilib options appear in the command line after the options
  specified with -mcpu and can result in using surprising or
  conflicting options for a particular test.

  We need a way to skip a test if the options needed to compile
  that test are not compatible with current multilib options.

This could get ugly, so I'd prefer to find a way to do the
necessary checks outside of the test itself, in a new directive.
If that sounds workable I'll look into it.

Janis

> Janis Johnson wrote:
> > On Fri, 2008-03-14 at 10:21 -0700, Janis Johnson wrote:
> >   
> >> On Fri, 2008-03-14 at 10:18 -0500, Joel Sherrill wrote:
> >>
> >> 
> >>> /* { dg-skip-if "" { *-*-* } { "-mcpu=405" } { "-mcpu=" } } */
> >>>
> >>> I think this is doing what we want it to. It looks like it results
> >>> the tests getting run when -mcpu=405 and excluded when
> >>> -mcpu=603e is set on the board cflags.
> >>>   
> >> The test directives are documented in the internals manual.  From
> >> sourcebuild.texi:
> >>
> >> @item @{ dg-skip-if @var{comment} @{ @var{selector} @}
> >> @{ @var{include-opts} @} @{ @var{exclude-opts} @} @}
> >> Skip the test if the test system is included in @var{selector} and if
> >> each of the options in @var{include-opts} is in the set of options with
> >> which the test would be compiled and if none of the options in
> >> @var{exclude-opts} is in the set of options with which the test would be
> >> compiled.
> >>
> >> The directive above says to skip the test on any target when
> >> "-mcpu=450" is used and "-mcpu=" is not used.  You'd actually
> >> want "-mcpu=*", since "-mcpu=" isn't a full option, but then
> >> the test will never be skipped whenever -mcpu=* is used.
> >>
> >> Proc check_conditional_xfail from framework.exp in DejaGnu
> >> processes the lists of included and excluded option and with
> >> "--verbose --verbose" in RUNTESTFLAGS will print messages to
> >> the test log (e.g. obj_dir/gcc/testsuite/gcc/gcc.log) about
> >> its progress.
> >>
> >> I think that dg-skip-if won't do what you need and you'll need
> >> to add effective-target keywords.
> >> 
> >
> > On the other hand, to run a test for -mcpu=450 but no other -mcpu
> > option, this very ugly directive works (pattern matching is done
> > with globs):
> >
> > /* { dg-skip-if "" { *-*-* } { "" } { "-mcpu=[0-35-9][0-46-9][1-9]*" } } */
> >
> > That kind of thing could get ugly very quickly if there are
> > multiple cpus that would work.
> >
> >   
> 

gcc-4.3-20080424 is now available

[gccadmin]

Snapshot gcc-4.3-20080424 is now available on
  ftp://gcc.gnu.org/pub/gcc/snapshots/4.3-20080424/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 4.3 SVN branch
with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_3-branch 
revision 134647

You'll find:

gcc-4.3-20080424.tar.bz2  Complete GCC (includes all of below)

gcc-core-4.3-20080424.tar.bz2 C front end and core compiler

gcc-ada-4.3-20080424.tar.bz2  Ada front end and runtime

gcc-fortran-4.3-20080424.tar.bz2  Fortran front end and runtime

gcc-g++-4.3-20080424.tar.bz2  C++ front end and runtime

gcc-java-4.3-20080424.tar.bz2 Java front end and runtime

gcc-objc-4.3-20080424.tar.bz2 Objective-C front end and runtime

gcc-testsuite-4.3-20080424.tar.bz2The GCC testsuite

Diffs from 4.3-20080417 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-4.3
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.

Re: dg-skip-if on powerpc when multiple cpu cflags specified

[Joel Sherrill]

Janis Johnson wrote:

On Wed, 2008-04-23 at 10:56 -0500, Joel Sherrill wrote:
  

Hi,

I am returning to this issue and it is more
pressing testing powerpc on 4.3.0 and the trunk.
powerpc-rtems has gone from a relatively small
percentage of failures to >8300 and this warning
shows up a lot (5120334 times)!

 Warning: /home/joel/work-gnat/svn/b-gcc1-powerpc/rtems_gcc_main.o uses
hard float,
/home/joel/work-gnat/svn/b-gcc1-powerpc/gcc/testsuite/gcc/20010124-1.x0
uses soft float

$ grep "hard float" ./gcc/testsuite/gcc/gcc.log | wc -l
5120334

The target is psim/603e.
rtems_gcc_main.o is the configuration file and RTEMS
initialization task setup for the gcc tests.
This file and the RTEMS library for were compiled with
hard float.

On this target, we really need to skip these tests to
get rid of all this noise.  I can't even begin to tell what
else is broken.

I had the impression you weren't happy with the code
you proposed and didn't sweep it in.   Is it an acceptable
solution?  Or do you all have another idea?



I don't think it's workable, but I understand that the
problem does require a solution.  

That was the impression before.  This is a real
problem that needs to be fixed.  It is just going
to be tricky to get right.

This is also a cross-target testing issue.  Other
architectures have similar tests but maybe not
quite as many as the PowerPC.

Is this a fair statement
of the problem?

  Multilib options can include -mcpu= or -march=.

  Many target-specific tests use dg-options to specify use
  of -mcpu= or -march= for that test.

  Multilib options appear in the command line after the options
  specified with -mcpu and can result in using surprising or
  conflicting options for a particular test.

  We need a way to skip a test if the options needed to compile
  that test are not compatible with current multilib options.
  

I think this is all true as stated.  Just to make sure we
are on the same page.  The specific boards (or simulators)
I am using to run the tests require specific CPU model
options.  Those may conflict with the specific -mcpu/-march
required by a test.

This could get ugly, so I'd prefer to find a way to do the
necessary checks outside of the test itself, in a new directive.
If that sounds workable I'll look into it.
  

Not knowing the internal details of the test harness, I
would make an ignorant guess that the command line
should be checked before it is executed.  If it has multiple
-mcpu/-march options and they were not all the same, the
test should be skipped.

Is that kind of the logic we need even if the implementation
is way off?

Thanks. 


--joel

Janis

  

Janis Johnson wrote:


On Fri, 2008-03-14 at 10:21 -0700, Janis Johnson wrote:

  

On Fri, 2008-03-14 at 10:18 -0500, Joel Sherrill wrote:




/* { dg-skip-if "" { *-*-* } { "-mcpu=405" } { "-mcpu=" } } */

I think this is doing what we want it to. It looks like it results
the tests getting run when -mcpu=405 and excluded when
-mcpu=603e is set on the board cflags.

  

The test directives are documented in the internals manual.  From
sourcebuild.texi:

@item @{ dg-skip-if @var{comment} @{ @var{selector} @}
@{ @var{include-opts} @} @{ @var{exclude-opts} @} @}
Skip the test if the test system is included in @var{selector} and if
each of the options in @var{include-opts} is in the set of options with
which the test would be compiled and if none of the options in
@var{exclude-opts} is in the set of options with which the test would be
compiled.

The directive above says to skip the test on any target when
"-mcpu=450" is used and "-mcpu=" is not used.  You'd actually
want "-mcpu=*", since "-mcpu=" isn't a full option, but then
the test will never be skipped whenever -mcpu=* is used.

Proc check_conditional_xfail from framework.exp in DejaGnu
processes the lists of included and excluded option and with
"--verbose --verbose" in RUNTESTFLAGS will print messages to
the test log (e.g. obj_dir/gcc/testsuite/gcc/gcc.log) about
its progress.

I think that dg-skip-if won't do what you need and you'll need
to add effective-target keywords.



On the other hand, to run a test for -mcpu=450 but no other -mcpu
option, this very ugly directive works (pattern matching is done
with globs):

/* { dg-skip-if "" { *-*-* } { "" } { "-mcpu=[0-35-9][0-46-9][1-9]*" } } */

That kind of thing could get ugly very quickly if there are
multiple cpus that would work.


  


  



--
Joel Sherrill, Ph.D. Director of Research & Development
[EMAIL PROTECTED]On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
  Support Available (256) 722-9985

Re: dg-skip-if on powerpc when multiple cpu cflags specified

[Janis Johnson]

On Thu, 2008-04-24 at 17:54 -0500, Joel Sherrill wrote:

> Not knowing the internal details of the test harness, I
> would make an ignorant guess that the command line
> should be checked before it is executed.  If it has multiple
> -mcpu/-march options and they were not all the same, the
> test should be skipped.
> 
> Is that kind of the logic we need even if the implementation
> is way off?

When you put it that way it sounds very simple!  Something like

/* { dg-options "-mcpu=xxx" } */
/* { dg-skip-if-incompatible-options } */

where dg-skip-if-incompatible-options starts out by looking for
multiple instances of non-identical -mcpu= or -march= and then
gets refined over time.  There's already code in check_flags in
target-supports-dg.exp to gather together all the various flags
that will be used for a test.

I'll investigate.

Janis 

Re: Security vulernarability or security feature?

[Prateek Saxena]

On Thu, Apr 24, 2008 at 2:20 PM, Ralph Loader <[EMAIL PROTECTED]> wrote:
> > I am very interested in seeing how this optimization can remove
>  > arithmetic overflows.
>
>  int foo (char * buf, int n)
>  {
> // buf+n may overflow of the programmer incorrectly passes
> // a large value of n.  But recent versions of gcc optimise
> // to 'n < 100', removing the overflow.
> return buf + n < buf + 100;
>  }

This clearly is insecure coding. The optimization to replace "buf + n
< buf + 100" with "n < 100" assumes something about the value of buf.
I assume that the location of "buf", could change arbitrarily accross
platforms depending on the memory layout.
I ran foo as follows, getting different outputs :

int main()
{
   printf ("%d\n", foo (0xbffc, 0x4010));
   return 0;
}

When "foo" is :
int foo (char * buf, int n)
{
  return buf + n < buf + 100;
}
Result : 1

When foo is (due to optimization, lets say) :
int foo (char * buf, int n)
{
 return n < 100;
}
Result : 0

When such assumptions are made ... the compiler may eliminate the bug
in some cases giving the programmer a false feeling that "Oh! My code
is bug free". The problem is that when the code is compiled on a
different platform, with different switches, the bug may reappear. I
just wanted to bring out the point about the assumption ... may be a
diagnostic should be issued. Or the compiler is smart enough to figure
out the values of "buf" and only optimise on cases which are safe.

>
>  Compiled on i386, gcc-4.3.0 with -O2 gives:
>
>  foo:
> xorl%eax, %eax
> cmpl$99, 8(%esp)
> setle   %al
> ret
>
>  E.g., calling foo with:
>
>  #include 
>  int main()
>  {
> char buf[100];
> printf ("%d\n", foo (buf, 15));
> return 0;
>  }
>
>  on my PC (where the stack is just below the 3Gig position).
>
>
>
>  > > Why is Cert advising people to avoid an optimisation that can ---
>  > > realistically, although probably rarely --- remove security
>  > > vulnerabilities?
>  > >
>  > If you are referring to VU#694123, this refers to an optimization
>
>  I'm talking about 162289.
>
>  Ralph.
>
>
>
>  > that removes checks pointer arithmetic wrapping.  The optimization
>  > doesn't actually eliminate the wrapping behavior; this still occurs.
>  > It does, however, eliminate certain kinds of checks (that depend upon
>  > undefined behavior).
>  >
>  > Thanks,
>  > rCs
>

Re: dg-skip-if on powerpc when multiple cpu cflags specified

[Joel Sherrill]

Janis Johnson wrote:

On Thu, 2008-04-24 at 17:54 -0500, Joel Sherrill wrote:

  

Not knowing the internal details of the test harness, I
would make an ignorant guess that the command line
should be checked before it is executed.  If it has multiple
-mcpu/-march options and they were not all the same, the
test should be skipped.

Is that kind of the logic we need even if the implementation
is way off?



When you put it that way it sounds very simple!  

It is easy to say. :-D

Something like

/* { dg-options "-mcpu=xxx" } */
/* { dg-skip-if-incompatible-options } */

where dg-skip-if-incompatible-options starts out by looking for
multiple instances of non-identical -mcpu= or -march= and then
gets refined over time.  There's already code in check_flags in
target-supports-dg.exp to gather together all the various flags
that will be used for a test.

  

This will involve editing every test that using dg-options
to add a -mcpu/-march flag.  Would it make sense to let
dg-options check for the conflict as it adds an option?

In my case, the options are added in the board file
this way:

set_board_info cflags  "-B${RTEMS_MAKEFILE_PATH}/lib/ -specs bsp_specs 
-qrtems -mcpu=603e"



I'll investigate.

  

Thanks. This is really a precision fix. :-D

Janis

  



--
Joel Sherrill, Ph.D. Director of Research & Development
[EMAIL PROTECTED]On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
  Support Available (256) 722-9985

Re: Security vulernarability or security feature?

[Ralph Loader]

> This clearly is insecure coding.

Yes, it was intended to be an example of what gcc does with bad code.

> When such assumptions are made ... the compiler may eliminate the bug
> in some cases giving the programmer a false feeling that "Oh! My code
> is bug free". The problem is that when the code is compiled on a
> different platform, with different switches, the bug may reappear.

Yes.  I was not intending to advocate reliance on the optimisation.  I
was attempting to illustrate that things are not as simple as Cert is
claiming.

> I
> just wanted to bring out the point about the assumption ... may be a
> diagnostic should be issued.

I believe this is being done as we talk.

Ralph.



 Or the compiler is smart enough to figure
> out the values of "buf" and only optimise on cases which are safe.
> 
> >
> >  Compiled on i386, gcc-4.3.0 with -O2 gives:
> >
> >  foo:
> > xorl%eax, %eax
> > cmpl$99, 8(%esp)
> > setle   %al
> > ret
> >
> >  E.g., calling foo with:
> >
> >  #include 
> >  int main()
> >  {
> > char buf[100];
> > printf ("%d\n", foo (buf, 15));
> > return 0;
> >  }
> >
> >  on my PC (where the stack is just below the 3Gig position).
> >
> >
> >
> >  > > Why is Cert advising people to avoid an optimisation that can
> >  > > --- realistically, although probably rarely --- remove security
> >  > > vulnerabilities?
> >  > >
> >  > If you are referring to VU#694123, this refers to an optimization
> >
> >  I'm talking about 162289.
> >
> >  Ralph.
> >
> >
> >
> >  > that removes checks pointer arithmetic wrapping.  The
> >  > optimization doesn't actually eliminate the wrapping behavior;
> >  > this still occurs. It does, however, eliminate certain kinds of
> >  > checks (that depend upon undefined behavior).
> >  >
> >  > Thanks,
> >  > rCs
> >

Re: Security vulernarability or security feature? VU#162289

[Robert C. Seacord]

Ralph,

Thanks for your further explanation of this optimization.  Here is what 
I understand.  Please correct me if I am wrong on any of these points:


1. The description in VU#162289 misrepresents the problem has a length 
check.  It is actually a check for wrap. 

2. The optimization in this code depends on the assumption that wrapping 
will not occur, or at the very least, that wrapping is undefined 
behavior so the compiler is not required to generate code for this 
condition.


3. Both expressions (buf+len < buf) and   buf + n < buf + 100 can be 
optimized assuming the subexpressions cannot wrap.


4. Both expressions cannot be optimized assuming modulo behavior, for 
example if buf were cast to uintptr_t, because there are values of buf 
and n where buf + n < buf + 100 evaluates to false and n <  100 
evaluates to true.


5. Both code examples are examples of poor code, that could be written 
better. 


The test for wrap would be better written as:

if ((uintptr_t)buf+len < (uintptr_t)buf)

And your expression should just be written as:

n <  100

6.  The Overview of VU#162289 states "Some C compilers optimize away 
pointer arithmetic overflow tests that depend on undefined behavior 
without providing a diagnostic (a warning)." 

The point being is that we are not objecting to the optimization, we are 
objecting to the lack of any diagnostic.


Because both examples could benefit from being rewritten (this is what 
the compiler is doing) I don't see the harm in issuing a diagnostic in 
both cases.


rCs


Robert,

You have failed to answer my original question, and I think have failed
to understand the point of the example.

The example shows that what you are claiming is a vulnerability in
162289 is in fact a security feature.

  

  that removes checks pointer arithmetic wrapping.



Just to be 100% clear, the optimisation I gave you an example of, and
which you think is "pretty cool" is precisely the same optimisation you
are complaining about in 162289, specifically, a pointer value may be
canceled from both sides of a comparison.

This is slightly confused by the fact that in the 162289 example, two
optimisations take place [gcc gurus please correct if I am wrong]:

(a) a pointer value is canceled from both sides of a comparison,
changing (buf+len  

 The optimization
doesn't actually eliminate the wrapping behavior; this still occurs.
It does, however, eliminate certain kinds of checks (that depend upon
undefined behavior).

Thanks,
rCs

Re: dg-skip-if on powerpc when multiple cpu cflags specified

[Janis Johnson]

On Thu, 2008-04-24 at 18:14 -0500, Joel Sherrill wrote:
> Janis Johnson wrote:
> > On Thu, 2008-04-24 at 17:54 -0500, Joel Sherrill wrote:
> >
> >   
> >> Not knowing the internal details of the test harness, I
> >> would make an ignorant guess that the command line
> >> should be checked before it is executed.  If it has multiple
> >> -mcpu/-march options and they were not all the same, the
> >> test should be skipped.
> >>
> >> Is that kind of the logic we need even if the implementation
> >> is way off?
> >> 
> >
> > When you put it that way it sounds very simple!  
> It is easy to say. :-D
> > Something like
> >
> > /* { dg-options "-mcpu=xxx" } */
> > /* { dg-skip-if-incompatible-options } */
> >
> > where dg-skip-if-incompatible-options starts out by looking for
> > multiple instances of non-identical -mcpu= or -march= and then
> > gets refined over time.  There's already code in check_flags in
> > target-supports-dg.exp to gather together all the various flags
> > that will be used for a test.
> >
> >   
> This will involve editing every test that using dg-options
> to add a -mcpu/-march flag.  Would it make sense to let
> dg-options check for the conflict as it adds an option?

Yes, it would meaning adding the new option to hundreds of tests,
but that's better than the earlier suggestion of adding a very
ugly dg-skip-if to every one of those tests.

I wouldn't want to change the meaning of dg-options to have it
start skipping tests.

> In my case, the options are added in the board file
> this way:
> 
> set_board_info cflags  "-B${RTEMS_MAKEFILE_PATH}/lib/ -specs bsp_specs 
> -qrtems -mcpu=603e"

Those options are in a data structure that directives like
dg-skip-if already look at.

> > I'll investigate.
> >
> >   
> Thanks. This is really a precision fix. :-D

Yeah, well, maybe a more elegant idea will arise to take its
place.

By the way, I won't have anything for a few days since I'll
be spending the next three days doing very non-computer stuff.

Janis

Re: Security vulernarability or security feature? VU#162289

[Ralph Loader]

> Thanks for your further explanation of this optimization.  Here is
> what I understand.  Please correct me if I am wrong on any of these
> points:

Points 1...5, no quibble.

Now that you appear to admit that the issue is wrap-around and not a
length-check, this raises the question of whether there is any security
problem with the optimisation at all, even for badly written
applications.

Length-checks are directly related to security, because they protect
against buffer-overruns which are often directly exploited by attackers.

It is much harder to see how reliance on wrap-around could contribute to
the security of an application.  Where pointer arithmetic is invalid,
and this is carried out in context where security matters, I would
expect it to not matter whether or not invalid arithmetic used
wrap-around or not.

Of course, for just about any compiler feature, it is probably possible
to contrive badly written code that is exploitably insecure with that
feature, and --- purely by chance --- not exploitable without the
feature.

But it is hard to see what reason you have for picking on this
particular feature of this particular compiler.

> 6.  The Overview of VU#162289 states "Some C compilers optimize away 
> pointer arithmetic overflow tests that depend on undefined behavior 
> without providing a diagnostic (a warning)." 
> 
> The point being is that we are not objecting to the optimization, we
> are objecting to the lack of any diagnostic.

Yes, you have dramatically improved the wording from previous
versions.  However, you still say:

  "avoid using compiler implementations that perform the offending
   optimization"

without any warning that in some cases the "offending" optimization may
be covering-up a security issues, and that avoiding the optimization
without fixing the underlying problems in the application may actually
be harmful.  (In fact, as far as I can, more likely to be harmful than
helpful).

I must admit, given the problems that have been identified with the
advisory (how many versions has it been through?), I would be far
happier if you subjected it to independent third-party expert review,
and withdrew the advisory until that is completed in a satisfactory
manner (rather than repeatedly incrementally tweaks).

Ralph.

> 
> Because both examples could benefit from being rewritten (this is
> what the compiler is doing) I don't see the harm in issuing a
> diagnostic in both cases.
> 
> rCs
> 
> > Robert,
> >
> > You have failed to answer my original question, and I think have
> > failed to understand the point of the example.
> >
> > The example shows that what you are claiming is a vulnerability in
> > 162289 is in fact a security feature.
> >
> >   
> >>   that removes checks pointer arithmetic wrapping.
> >> 
> >
> > Just to be 100% clear, the optimisation I gave you an example of,
> > and which you think is "pretty cool" is precisely the same
> > optimisation you are complaining about in 162289, specifically, a
> > pointer value may be canceled from both sides of a comparison.
> >
> > This is slightly confused by the fact that in the 162289 example,
> > two optimisations take place [gcc gurus please correct if I am
> > wrong]:
> >
> > (a) a pointer value is canceled from both sides of a comparison,
> > changing (buf+len > observable behaviour of the code, and is what the debate is about.
> >
> > (b) in the example given in 162289, (len<0) can then be evaluated at
> > compile time, removing the test entirely.   This does not change the
> > runtime behaviour of the code an is completely irrelevant.
> >
> > To make it even clearer, we can disable optimisation (b) while
> > leaving optimisation (a), by making 'len' volatile:
> >
> > int foo (char * buf)
> > {
> > volatile int len = 1<<30;
> > if (buf+len < buf)
> > return 1;
> > else
> > return 0;
> > }
> >
> > gcc then generates:
> >
> > foo:
> > subl$16, %esp
> > movl$1073741824, 12(%esp)
> > movl12(%esp), %eax
> > addl$16, %esp
> > shrl$31, %eax
> > ret
> >
> > This has not completely removed the test, but when executed, it will
> > still always return 0, giving precisely the same run-time behaviour
> > as without the 'volatile'.
> >
> > To re-iterate:
> >
> > (a) Why does Cert advise against an optimisation that, under
> > the right circumstances, can remove examples of a historically
> > significant class of security holes.
> >
> > (b) The example you claim is a length check in 162289 is not a
> > length check and does not support the conclusions you draw from it.
> >
> > Ralph.
> >
> >
> >   
> >>  The optimization
> >> doesn't actually eliminate the wrapping behavior; this still
> >> occurs. It does, however, eliminate certain kinds of checks (that
> >> depend upon undefined behavior).
> >>
> >> Thanks,
> >> rCs
> >> 
> >> 
> 

Re: IRA for GCC 4.4

[Vladimir Makarov]

Peter Bergner wrote:

On Thu, 2008-04-24 at 16:33 -0500, Peter Bergner wrote:
  

On Thu, 2008-04-24 at 16:51 +0200, Paolo Bonzini wrote:


(The testcase is 400k lines of preprocessed Fortran code, 16M is size,
available here:
http://www.pci.unizh.ch/vandevondele/tmp/all_cp2k_gfortran.f90.gz)

  
  

Thanks, I'll check it.

Vlad, I think you should also try to understand what does trunk do with 
  global (and without local allocation) at -O0.  That will give a 
measure of the benefit from Peter's patches for conflict graph building.
  

I took a patch from Ken/Steven that disabled local_alloc and instead runs
global_alloc() at -O0 and summing up all of the bit matrix allocation
info we emit into the *.greg output, the new conflict builder saves a lot
of space compared to the old square bit matrix (almost 20x less space).
Here's the accumulated data for the test case above:

compressed upper triangular:  431210251 bits,   53902848 bytes
upper triangular:4264666581 bits,  533084851 bytes
square:  8531372796 bits, 1066423618 bytes



The SPEC2000 numbers look even better (29x less space):

compressed upper triangular:  281657797 bits,   35212532 bytes
upper triangular 4094809686 bits,  511856604 bytes
square:  8191641644 bits, 1023962188 bytes
  


Hi, Peter.  The last time I looked at the conflict builder 
(ra-conflict.c), I did not see the compressed matrix.  Is it in the 
trunk?  What should I look at?


I have also another question.  I saw that sparset was used for the 
conflict builder.  I tried that too when I worked on YARA project.  I 
even wanted to contribute a generic sparset implementation.  But I found 
that in general case bitmaps are not worse the sparse sets and much 
better if we take  a needed space into account.  May be you have another 
impression?  It would be very interesting for me to hear it.  I found 
that bitmaps have more advanced design than sparsets.  I always wanted 
to find inventors the bitmaps but never tracked them down.


Vlad

Re: Security vulernarability or security feature? VU#162289

[Robert C. Seacord]

Ralph,

Comments below.

Length-checks are directly related to security, because they protect
against buffer-overruns which are often directly exploited by attackers.

It is much harder to see how reliance on wrap-around could contribute to
the security of an application.  
The original impetus for this came from a check in a sprint() function 
from Plan 9.  Because of the API, there was no way to test if the len 
was out of bounds, but the developers wanted to make sure they weren't 
wrapping the stack on some architectures that have their stacks in high 
memory.


It seems like a reasonable test to make, the code used to work, and they 
got burned on the silent change in behavior.

But it is hard to see what reason you have for picking on this
particular feature of this particular compiler.
  
This may not have been the poster child issue to go after, but we were 
responding to the report.


What features of which compilers do you think we should be reporting on?

Yes, you have dramatically improved the wording from previous
versions.  However, you still say:

  "avoid using compiler implementations that perform the offending
   optimization"
  

I agree this is worded badly, we'll correct this.

I must admit, given the problems that have been identified with the
advisory (how many versions has it been through?), 
we've only revised it once so far, but we'll change it as many times as 
we need to.

I would be far
happier if you subjected it to independent third-party expert review,
  
we are an independent third party.  who else would we ask? 

and withdrew the advisory until that is completed in a satisfactory
manner (rather than repeatedly incrementally tweaks).
  
withdrawing vulnerability notes (this is not an advisory that is emailed 
out) is not as good a solution as you might think, as this usually draws 
more attention to the vulnerability note than just about anything else 
you can do.


rCs

Re: US-CERT Vulnerability Note VU#162289

[Paul Schlie]

Mark Mitchell wrote:

> ...
>
> And:
>
>> Addition or subtraction of a pointer into, or just beyond, an array object
>> and an integer type produces a result that does not point into, or just
>> beyond, the same array object (6.5.6).
>>
>> is undefined behavior.

So then unless the compiler can determine that all pointers passed to foo,
for example below, represent a pointer to some Nth element of some array
from which not more than N will be effectively subtracted, the optimization
(if it can be called that) can not be performed, as a sum of a pointer and
an arbitrary integer may be validly be less than said pointer, as follows:

foo(char* p){
if (p+(char*)-1 < p)
 }

char a[] {0,1,2,3};

foo(a+2);

I believe.

Re: US-CERT Vulnerability Note VU#162289

[Paul Koning]

> "Paul" == Paul Schlie <[EMAIL PROTECTED]> writes:

 Paul> Mark Mitchell wrote:
 >> ...
 >> 
 >> And:
 >> 
 >>> Addition or subtraction of a pointer into, or just beyond, an
 >>> array object and an integer type produces a result that does not
 >>> point into, or just beyond, the same array object (6.5.6).
 >>> 
 >>> is undefined behavior.

 Paul> So then unless the compiler can determine that all pointers
 Paul> passed to foo, for example below, represent a pointer to some
 Paul> Nth element of some array from which not more than N will be
 Paul> effectively subtracted, the optimization (if it can be called
 Paul> that) can not be performed, as a sum of a pointer and an
 Paul> arbitrary integer may be validly be less than said pointer, ...

I think you're misinterpreting what "undefined" means.

The C standard doesn't constrain what a compiler does for "undefined"
programs -- it only imposes requirements for programs that do not do
undefined things.

So an optimization that "does what I mean" only for "defined" cases,
but does something surprising for "undefined" cases, is a valid
optimization.  The compiler is under NO obligation to check if you've
done something undefined -- instead, it is allowed to assume that you
are NOT doing undefined things.

So, for example: pointer arithmetic in programs that obey the C
standard cannot cause overflow, or arithmetic wrap, or things like
that.  Only "undefined" pointer arithmetic can do that.  Therefore the
compiler is allowed to make optimizations that "do what I mean" only
for the no-wrap or no-overflow cases.

As others have pointed out, programs that contain undefined actions
are broken, and any security issues caused are the fault of those
programs.  The compiler is NOT at fault.

That said, it certainly is helpful if the compiler can detect some
undefined actions and warn about them.  But that doesn't create a duty
to warn about all of them.

   paul

Re: Security vulernarability or security feature? VU#162289

[Joe Buck]

On Thu, Apr 24, 2008 at 08:37:43PM -0400, Robert C. Seacord wrote:
> The original impetus for this came from a check in a sprint() function 
> from Plan 9.  Because of the API, there was no way to test if the len 
> was out of bounds, but the developers wanted to make sure they weren't 
> wrapping the stack on some architectures that have their stacks in high 
> memory.

It sounds like there are two issues here: a problem with the API (ideally
it would be possible to do a proper length check, though sometimes these
are legacy issues that one is stuck with), and a problem with the way the
test is coded.  A portable-in-practice way to do checks on pointer
addresses of this kind is to cast the pointers to unsigned integers of the
appropriate length before doing the arithmetic.  In C, unsigned types are
guaranteed to obey the rules of modulo 2**N arithmetic, where N is the
number of bits.

(I say "portable in practice" because things like segmented architectures
raise other issues, but for a flat address space on a conventional
processor that has an integral type the same size as a pointer type,
you should be OK).

> It seems like a reasonable test to make, the code used to work, and they 
> got burned on the silent change in behavior.

Unfortunately, C is not designed that way, which makes it rather
surprising that Plan 9 was burned by this (it has architects who should
know better, given their role in the definition of C).  These are not
new issues; they were fought over back in the late 1980s, and we have
the rules we do because otherwise, anyone wanting to write fast code
would need to use Fortran, as the "consistency" you are asking for
would mean that everything aliases everything so it's unsafe to use
registers.

Re: Help me with the patterns

[Mohamed Shafi]

On Thu, Apr 24, 2008 at 12:01 PM, Mohamed Shafi <[EMAIL PROTECTED]> wrote:
> Hello all,
>
>  The target that i am porting in gcc 4.1.2, has the following instructions
>
>  setb Rx, bitno
>  clrb Rx, bitno
>
>  where bit bitno of Rx will either be set or reset.
>
>  For statements like
>
>  a |= (1 << 2); and
>  b &= ~(1 << 2);
>
>  I can use the above instructions directly. But i am not sure how to
>  write the patterns for the above instructions.
>  Can anybody help me with this?
>

What i have done is to have the following pattern

(define_insn "setbhi3"
  [(set (match_operand:HI 0 "register_operand" "=r")
(ior:HI (match_operand:HI 1 "register_operand" "%0")
(match_operand:HI 2 "setclrb_operand" "I")))]
  ""
  "setb\\t%0, %2"
)
 where setclrb_operand is

(define_predicate "setclrb_operand"
  (match_code "const_int")
{
  if (CONSTANT_P (op))
  {
if (INTVAL(op) && !(INTVAL(op) & (INTVAL(op) - 1)))
  if (INTVAL(op) < BITS_PER_WORD)
return 1;
  }

  return 0;
})

I am now able to generate the instruction.
Is this OK?

Thank you for your time.

Regards,
Shafi.