This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [egcs 1.1] Fix gcc.c handling of %O vis-a-vis temp files



  In message <199807271939.PAA09021@melange.gnu.org>you write:
  > >  > The bug I'm fixing is that the handling of `%g%O' is broken.  It is
  > >  > *supposed* to be exactly as if the proper target-specific suffix
  > >  > (normally `.o') is present, as in `%g.o'.  But, unlike when `.o' is
  > >  > explicitly used, do_spec_1 currently generates a *new* filename per
  > >  > *occurrence* of `%g%O' in the specs it is processing.
  > >OK.  Is it %g or %O that is causing us to create a new filename?
  > 
  > It's %g's handling of %O.  The code is clearly trying to *avoid*
  > creating a new filename when the suffix is one for which a
  > filename has already been created.  The code to do this is simply
  > broken for the case of %O.
OK.  Thanks for clarifying many things.

  > I still feel my explanation was, while short, sufficient such that,
  > in combination with looking at the docs for the relevant bits,
  > it could be clearly understood as a fix rather than a change in
  > the design.
Unfortunately, I can't read the related code for every patch in detail.
So generally when I get patches from trusted contributors like
yourself I only worry about the higher level issues (ie are we going
the right direction) rather than the low level bits which I trust you
to sort out.  That way I can spend more time helping newer folks or
debugging code gen issues which I understand much better than most
other parts of the compiler.

As a result it is not uncommon for me to receive a patch from someone
like yourself and make some comments about it, which after a couple
quick emails are easily tossed aside as non-issues or which have already
been suitably dealt with.

This is quite different from how gcc has been developed in the past, and
it is going to have some bumps along the way.  It may even be the wrong
approach to take with patches -- *I don't know*.  Experimenting with
the development model to improve the development environment is one
of the goals of the project.  And I'm strong believer that you have
to make mistakes sometimes to achieve significant improvements.


So when you get these seemingly "what the (*&@#$ is Jeff thinking"
questions, keep in mind that I may not be familiar with the code or
the details of the problem you're trying to solve.  I'm just trying
to make sure we're doing the right thing at a high level.

More than once in this message you stated that I appeared to be willing
to "toss g77 -v overboard".  That is not the case.  I'm just trying to
make sure we're doing the right thing both in gcc.c and g77spec.c.  I
don't want to see us throw away g77 -v or the security improvements
we've made to the compiler.


  > Perhaps the fact that whoever changed the behavior of
  > %g didn't also change the commentary documenting %g would cause
  > confusion
I think the problem is when I made the changes to the temporary file
handling I was not aware they were going to have this particular side
effect.  Or maybe it was Nick's recent change.  Either way I wasn't
aware that they were going to have this kind of effect on how %g works
and thus I wasn't aware that we needed a doc change elsewhere in gcc.c

Actually, I suspect it was Nick's change as I thought g77 -v was working
at some point after I made the original changes to temporary file handling.

  > >I would claim that using %g is a bad thing because it creates easily
  > >predictable names (since it's supposed to be choosen once per
  > >compilation) which leads to a denial of service attack.
  > 
  > Actually, %g no longer chooses a name once per *compilation*.  It
  > chooses a single name *per suffix* per compilation.  That's the
  > point of the recent changes (that broke `%g%O', unfortunately).
OK.  Again, it's obvious I don't have a solid understanding of this
code, so thanks for clarifying this issue (and others).

  > And, clearly, after you write assembly code to a file named `foo.s',
  > you can read that file (that is, get the assembler to process it)
  > *only* by opening a file named `foo.s'.
I would object to "*only*" since the toplevel driver could pass around
file descriptors to sub-processes :-) :-0  We might even want to do
this at some point to close some of the denial of service attacks which
remain in the compiler.  But that's a side issue we can ignore for now.

  > And my patch, which fixes `%g%O', doesn't affect that at all.
  > 
  > In summary: you're complaining that *my* patch wasn't sufficiently
  > described, but IMO it was, and the fault was that somebody, somewhere,
  > applied patches changing the behavior of %g without sufficiently
  > describing *and documenting* them.
And that misunderstanding may be because the Nick's patch did not
fix the docs as it should have which led to some misunderstandings
on my part about what %g was supposed to do.

Remember I don't have all the state you do on the problem, so when I
went back and read some of the docs for this stuff I was actually
reading incorrect information and I didn't have the correct info
handy.

  > confusion now.  (IMO, not on my part, but it does seem that the docs
  > for %g are confusing, or at least troubling, you in a way that the code
  > perhaps would not.)
Agreed.


  > Also, if you're willing to finally apply my patch, I'm willing to
  > submit another patch to fix the commentary that describes %g/%u/%U/%O.
  > That should be done anyway; but my patch would clarify that way I
  > believe it is *supposed* to work, so there's no point in my submitting
  > it unless you first accept my previous patch.  (Otherwise somebody
  > should fix the commentary to document the IMO-buggy way of doing things.)
If you could send this patch too, it would be greatly appreciated.  I
strongly believe that part of the confusion at hand is caused because
I read the incorrect docs in gcc.c (instead of the code).

At this point I do not see a reason why your change should not go in.

jeff




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]