Re: Commments on 2.1 draft 1 (A)

[brender at gemevn dot zko dot dec dot com (Ron 603-884-2088)]

Dave,

Thanks much for this *great* set of review comments. Most of them are
now dealt with, although not always quite as you suggest (of course).
Here is a brief summary of my actions. Key:

  YES or OK => I agree and did something close to the lines suggested.
  NO => Disagree, reason given.
  PENDING => Still pending action, reason possibly given.

>The following are my comments on the April 5 2000 draft 1.
>
>The document is quite nice, but there are a few things that, I think,
>need attention (beyond Ron's TBD-marked areas).
>
>I considered writing each of these as a separate email, but
>that became rather a long list (would you want that many 
>pieces of email at once?).
>
>Some of these are simply matters of editorial taste
>(so others will likely disagree).
>Some are a bit stronger than just 'taste'.
>Some are matters of correctness.
>
>No doubt Ron is aware of some or all of these already, but
>I wade in anyway.

>===CONTENTS, page iv
>I cannot find an entry here for "extended-section-offset"
>(or whatever we finally call it).  See below where I
>suggest this term instead of 64 bit safe.
>But my point here is that nothing about this shows in the TOC.
>(see later for more on this, sec 7.4)

PENDING -- how to handle is part of 000403.2 matters.

>====FOREWORD page viii
>"but has additional links"
>is some kind of jargon. I'm not sure exactly what is meant by 'links'.
>Lets not use such a term here (it's not html).

OK. "Links" (actually now "hyperlinks") now explained.

>==== 2.4 dwarf expressions, page 11
>"In DWARF Version 2, all DWARF"  should be
>"In DWARF Version 2.0.0, all DWARF" I think.

NO. The DWARF V2 document refers to itself in several places as "Version 2".
So long as we identify Version 2 by date early on (now done) I think we are
fine.

>==== 2.4.2.5 ControlFlow Operations  page 18
>
>In the OP_call descr:
>"of a DIE in the current unit" should be
>"of a DIE in the current compilation unit"  I think

YES

>"Execution of the DW_AT_location attribute" should be
>"Execution of the DWARF expression of the  DW_AT_location attribute" 
>
>and later in that para. 'called attribute' should be
>	'called DWARF expression' in two places, I suggest.

YES

>==== 2.13 Declaration coordinates page 26, also page 79
>
>"statement information table" is  mentioned a couple times, but
>that phrase is not used at the definition of the line table.
>That is, there is a lack of consistency here in the
>way the line/statement information is mentioned. This
>lack of consistency is not a good thing, IMO.
>
>This consistency problem is not new AFAIK. But
>removing the inconsistencies and using a single terminology
>would be a good idea, I think.

Pending (definitely not new). 000403.3 addresses at least in part.

>==== 3.1 Compilation Unit Entries page 30
>The description DW_AT_base_types is still not clear.
>
>It's useful for interpreting a conversion to a base type 
>when that base type is not mentioned in a current compilation
>unit, as then there is no reference chain for a debugger to follow,
>so a commonized list of base types  off in some
>other compilation unit might help.
>
>It's also not clear (any more than in the original document)
>what DIE type this might point to. CU die? Something else?
>Did we ever decide this for the original document?
>Am I just missing something here?

PENDING -- I don't adequately understand your concern...

>=== 5.4 array type entries, page 48
>It would be good here to make a forward reference to
>5.13, Dynamic Type Properties, and DW_AT_data_location.
>Not to repeat anything here from 5.13, 
>just as a hint there is more available that is described later.

OK

>====5.4 array type entries, page 49.
>
>"Note that if the size"...
>is in italics and the placement makes the following read better, IMO:
>"If the size"...

OK

>The "Note that" here are just noise words here and add nothing. IMO.
>(Not condemning "Note" in general...)

OK

>==== 5.5.4 Structure Data Member Entries  p 51
>
>It seems odd to me that 5.5.2 specifically mentions
>DW_AT_accessibility, whereas 5.5.4 does not.  5.5.4 should
>mention it too, as it is very meaningful there for C++.

YES

>==== 5.11 pointer to member type entries p 60
>"Finally, the pointer to member entry has a DW_AT_use_location
>attribute whose value is a location description that computes
>the address of the member of the class or structure
>to which the pointer to member entry can point".
>
>The above makes no sense. Well, less than it should. I think
>the intended meaning is expressed by:
>
>"Finally, the pointer to member entry has a DW_AT_use_location
>attribute whose value is a location description that computes
>the address of the member of the class or structure
>to which the pointer to member entry points (the value
>is meaningless if the pointer-to-member does not currently
>point at anything).".

YES

>But maybe I'm just confused???

Me too by that phrase...

>====5.13.2 allocation and association status  pg 62
>
>"may then optionally be" should be
>"may then be"  I think.

OK

>====6.1.1 lookup by name pge 63
>
>"the names of global objects whose definitions or declarations"
>is misleading if you think of 'objects' as in the C standard or
>C++ standard.
>
>
>"the names of global objects or functions whose definitions or declarations"

PENDING your proposal.

>====6.1.1 lookup by name pge 64
>"the name of a static data member or function member of a C++ structure" 
>should be
>
>"the name of a static-data-member or function-member of a C++ structure"  
>
>as otherwise it's unclear if 'static' should apply
>to the 'function' part.

YES (I used "function member or static data member" -- just reversed order
to resolve ambiguity)

>I worry (as Ron did) about the use of 'declaration' in 6.1.1
>and do not understand why a declaration would usefully appear here.
>
>Is it just Ron and I?


>==== 6.2.4 statement program prologe page 69
>
>This is (I think) the
>first appearance of '64-bit safe DWARF' and, like
>Mike, I think this is the wrong phrase to use.
>It's not accurate.
>
>After pondering for a few minutes, I think that
>  "extended-section-offset DWARF"
>would be a better description.
>(not perfect, but better)
>
>The phrase appears many places of course.

PENDING -- part of 000403.2 discussion. How do

    32-bit DWARF format
and
    64-bit DWARF format

grab ya for the original and our/your extension?

>====7.5.4 attribute encodings page 94
>
>See the paragraph beginning "There are two types of reference."
>
>"There are five forms for this type of reference: one, two, foru, and 
>eight byte offsets (respectively DW_FORM_ref1, DW_FORM_ref2,
>DW_FORM_ref4, and DW_FORM_ref8). There is also an unsigned
>variable length offset encoded form that
>uses LEB128 numbers (DW_FORM_ref_udata).
>This offset is relocated (non-relocatable) in all forms of object file.
>
>The above has various problems (that last sentence makes no 
>sense to me... Ah. Now I perhaps sense its meaning.)
>
>Note that some of the problems (lack of parallelism, for example)
>are in the original 2.0.0 text as well.
>
>I would suggest, instead.
>
>"There are five forms for this type of reference.
>DW_FORM_ref1, DW_FORM_ref2, DW_FORM_ref4, DW_FORM_ref8,
>and DW_FORM_ref_udata.  Because the reference is
>within  a single compilation unit no relocation
>of the value is required."

YES

>====7.5.4 attribute encodings page 94
>The paragraph beginning just after the above, 
>"The second type of reference"
>
>mentions "an offset from the beginning of the .debug_info section".
>I was wondering if (this is a general comment) all references
>to 'offset' should be restated as
>	offset or extended-section-offset
>
>Actually it might be better to have a definition of terms,
>and define offset briefly there as meaning both the
>basic 4 byte offset and extended-section-offset
>as appropriate for a given compilation unit.

PENDING 000403.2

>====7.5.4 attribute encodings page 94
>
>Mentions 4-byte offset. One of many places in the document...

PENDING 0000403.2

>====Figure 32, page 116.
>
>Omits DW_LNS_set_prologue_end DW_LNS_set_epilogue_begin.

YES

>====7.2.4 Dependencies, page 119
>
>"In target architectures with 64-bit addresses, the
>assembler and linker must similarly handle 8-byte references
>at arbitrary alignments."
>
>Might be more complete as:
>
>"In target architectures with 64-bit addresses the
>assembler and linker must similarly handle 8-byte references
>at arbitrary alignments (extended-section-offsets also require
>8 byte reference relocations)."

YES (applies to 7.24 actually, not 7.2.4)

>====Appendix 2, Organization of Debugging Information page 143
>
>This draft printout of this section is unfortunately
>unintelligible.  It needs to get back onto one page,
>properly laid out as per the v2.0.0 document.

YES. I actually managed to get it all back on one page! But,
what I did has the side-effect of reseting the page number
counter. Be patient while I try to beat Word 97 back into submission...

>====Appendix 3, Statement Program Examples, page 147.
>
>The print here is too big. Unless one gets the whole
>works onto one page the result is unreadable, as
>one must look back and forth many times in studying
>the example.  Get it all on one page if at all possible.
>Or at least get the beginning plus the first encoding
>on one page. But preferably all.

YES -- is much more like original now.


>==== Appendix 6 Aggregate examples, page 155
>
>First paragraph, f90 example:
>
>"necessarily longer (includes) that of the raw data"
>
>would really be, IMO
>
>"necessarily longer than and includes that of the raw data,"

YES

>Second paragraph:
>
>"2) there doesn't even need"
>
>might be better as
>
>"2) there doesn't need"

YES

>==== Appendix 6 Aggregate examples, page 156
>
>Just before the paragraph beginning "The F90 derived type array_ptr"
>I suggest adding a paragraph:
>
>"If an object has a descriptor, then the dwarf type for the object
>will have the DW_AT_data_location attribute. If an object
>does not have a descriptor, then the dwarf type for the object
>will not have the DW_AT_data_location attribute."
>
>This is to reinforce what I think Ron means...

YES (almost)

>==== Appendix 6 Aggregate examples, page 156
>
>Change
>	"fixed compiletime known size"
>to
>	"fixed compile time size"
>, IMO.

YES

>Note that the "desc<1>" sentence that this refers to is
>one that I had to stare at quite a while before I began
>to understand it.  It could just be me or it could be
>that this needs some rephrasing or elaboration?

PENDING -- seems OK to me, and I have yet to think of an
alternative I like... Is it just a matter of defining before
use, rather than versa-vica?

>==== Appendix 6 Aggregate examples, page 157
>
>In several places there is
>	DW_OP_push_object_address
>	DW_OP_deref
>	DW_OP_lit0
>	DW_OP_ne
>I think this is incorrect.
>I think these all should be
>	DW_OP_push_object_address
>	DW_OP_deref
>	DW_OP_lit1     !value corresponding to bit position
>		       ! of the associated flag, to mask
>		       ! so we only test the 'associated' bit.
>	DW_OP_and
>	DW_OP_lit0
>	DW_OP_ne

YES
	
>==== Appendix 6 Aggregate examples, page 157, etc
>
>DW_OP_add appears a few times. Should be DW_OP_plus

YES

>==== Appendix 6 Aggregate examples, page 157, etc
>
>I think the TAGS should be slightly indented where
>appropriate to indicate child (vs sibling) relationship.
>The existing layout obscures this essential feature of
>DW_TAG_subrange_type, for example.

YES (but I wasn't sure specifically which you were concerned about,
so do doublecheck that my fixes got what you saw)

>==== Appendix 6 Aggregate examples, page 160
>
>"This is all just like step 2), so the details are omited.
>Suffice it to note that the object address of interest here
>is the address that resulted from step 4."
>
>I believe this is correct, but I think it should be rephrased.
>(aside from the misspelling of omited[sic]).
>
>It does not emphasize enough the key things of interest.
>a) that because the type ($1) has DW_AT_data_location 
>the object address we get from step 4 is a descriptor object.
>This is a strong statement about how the code generator works
>(this is also a requirement for correct code generation,
>I believe, but it is nontheless a strong statement about
>the generated code).
>
>b) The DW_OP_push_object_address in 1$ and 2$ refer to this object
>which is, per the definition, a descriptor.
>
>In other words, I'm asking for a more thorough explanation
>here, to forestall confusion.

YES

>=== extended-section-offset
>or whatever we call it.
>
>Now I cannot find the text describing this (just mentions
>of the 64bit-safe term (which, as I said, I don't like))..
>
>Ah. I found this searching for 'safe' and it's TBD.
>Section 7.4.    But the name does not relate to
>the other terminolgy, so I think 7.4 should be
>called extended-dwarf-offset or whatever
>we finally call this, so the TOC has something that
>relates directly to the text.
>
>We need to make clear in 7.4, that
>this is  necessary to allow dwarf sections to grow
>over 4GByte, and that it allows mixing of 32bit offsets
>(from some compilation units) 
>with extended-section-offsets (from other compilation units).
>But that the only guaranteed way of being
>able to create and link sections over
>4GByte in size
>is for all compilation units in that section of that
>object to have extended-section-offsets.

PENDING 000403.2

Again, Dave, thanks much!
Ron