sarif — archive

Re: [sarif] #401: Improved design of address object design

From: Jim Kupsch <[email protected]>
To: [email protected]
Date: Mon, 29 Apr 2019 18:30:00 -0000

I think this alternative complicates the math for consumers, a boolean name unknownLength and unknownOffset would be better choices if there can't be a null value. The default value could be the length not equal to the default length or offset (-1 would be good default). Then length and offset can take any value, negative or positive. Jim On 4/29/19 12:45 PM, Larry Golding (Myriad Consulting Inc) wrote: > As an alternative to allowing lengthand offsetFromParentto be negative, > we could define a property growthDirection(or even just direction) > values "positive"and "negative", default value "positive". If it s > "negative", then both offsetFromParentand lengthare interpreted as > negative numbers. > > I m not 100% happy with that either it would be too easy for an SDK > consumer to add offsetFromParentto its parent address without noticing > it should have reversed the sign. > > *From:*Larry Golding (Myriad Consulting Inc) <[email protected]> > *Sent:* Monday, April 29, 2019 10:25 AM > *To:* Larry Golding (Myriad Consulting Inc) <[email protected]>; > James Kupsch <[email protected]>; Michael Fanning > <[email protected]>; OASIS SARIF TC Discussion List > <[email protected]> > *Subject:* RE: [sarif] #401: Improved design of address object design > > Another question: What does a lengthof zero mean? Is it an insertion > point , a zero-length range preceding the location specified by > offsetFromParent/absoluteAddress/relativeAddress? > > *From:*[email protected] < mailto:[email protected] > > <[email protected] < mailto:[email protected] >> *On > Behalf Of *Larry Golding (Myriad Consulting Inc) > *Sent:* Monday, April 29, 2019 10:18 AM > *To:* James Kupsch <[email protected] < mailto:[email protected] >>; > Michael Fanning <[email protected] > < mailto:[email protected] >>; OASIS SARIF TC Discussion List > <[email protected] < mailto:[email protected] >> > *Subject:* RE: [sarif] #401: Improved design of address object design > > Sorry, I shouldn t have directed those questions only to Michael. It was > just because he proposed the original redesign in #403. Of course I want > to hear feedback from the entire TC. > > *From:*Larry Golding (Myriad Consulting Inc) <[email protected] > < mailto:[email protected] >> > *Sent:* Monday, April 29, 2019 10:16 AM > *To:* Larry Golding (Myriad Consulting Inc) <[email protected] > < mailto:[email protected] >>; James Kupsch <[email protected] > < mailto:[email protected] >>; Michael Fanning > <[email protected] < mailto:[email protected] >>; > OASIS SARIF TC Discussion List <[email protected] > < mailto:[email protected] >> > *Subject:* RE: [sarif] #401: Improved design of address object design > > Another concern I have is around length. I want to be able to write that > if an addressobject occurs within a physicalLocationobject thisObject, > and if thisObject.lengthand thisObject.address.lengthare both present, > then then *SHALL* be equal. > > As the spec stands, I can t write this because physicalLocation.lengthis > non-negative. I can add words like this: > > If an addressobject occurs within a physicalLocationobject thisObject, > and if thisObject.lengthand thisObject.address.lengthare both present, > then then *SHALL* be equal. In that case, if thisObject.address.lengthis > negative, then thisObject.lengthwill also be negative. In all other > circumstances, lengthSHALL be non-negative. > > *Michael*, are you ok with that complication? > > Larry > > *From:*[email protected] < mailto:[email protected] > > <[email protected] < mailto:[email protected] >> *On > Behalf Of *Larry Golding (Myriad Consulting Inc) > *Sent:* Monday, April 29, 2019 10:08 AM > *To:* James Kupsch <[email protected] < mailto:[email protected] >>; > Michael Fanning <[email protected] > < mailto:[email protected] >>; OASIS SARIF TC Discussion List > <[email protected] < mailto:[email protected] >> > *Subject:* RE: [sarif] #401: Improved design of address object design > > Jim, > > From an SDK standpoint we have a problem with negative values because > without -1 as a sentinel value, we need to make the .NET type > Nullable<int>instead of int. So far we have avoided nullables in the > API. Our code generation doesn t currently support them, but it wouldn t > be hard to add. > > *Michael*, could you weigh in on this? The .NET Framework Design > Guidelines ( 8.8) say: > > *CONSIDER*using Nullable<T>to represent values that might not be present > (/i.e./, optional values). > > ** > > and a quick web search doesn t turn up recommendations to avoid them > in this scenario. > > I d just be concerned that this is the only place we have nullables. Are > there alternatives? > > In the meantime I ll write the words for the rest of Jim s feedback. > > Larry > > *From:*James Kupsch <[email protected] < mailto:[email protected] >> > *Sent:* Monday, April 29, 2019 8:40 AM > *To:* Michael Fanning <[email protected] > < mailto:[email protected] >>; Larry Golding (Myriad > Consulting Inc) <[email protected] < mailto:[email protected] >>; > OASIS SARIF TC Discussion List <[email protected] > < mailto:[email protected] >> > *Subject:* Re: [sarif] #401: Improved design of address object design > > For #2,I was thinking the "stack" kind would be the whole stack. Also > having a "stackFrame" kind would also be useful as it also a defined > memory region contained within the "stack" kind. > > JIm > > On 4/29/19 10:20 AM, Michael Fanning wrote: > > Right now, the effective address is absolute if unparented and relative > to some other thing, if parented. > > If you chase to the root object, let s say it s a module, its effective > address provides the ability to distinguish a child s relative address > vs. physical memory location. > > IOW, the child s relative address is its offset + all offsets up to (but > not including) the root object. If you add the effective address of the > root object, you get the absolute memory address (if available). > > Make sense? What do you think? > > For #2 stack, is the kind a stack frame? > > For #3, I see the problem. Every address in a PLC has an associated > region but the run.addresses cache does not provide this association. > Let me have a word with Larry. > > *From:*[email protected] < mailto:[email protected] > > <[email protected]> < mailto:[email protected] > *On > Behalf Of *James Kupsch > *Sent:* Monday, April 29, 2019 8:06 AM > *To:* Larry Golding (Myriad Consulting Inc) <[email protected]> > < mailto:[email protected] >; OASIS SARIF TC Discussion List > <[email protected]> < mailto:[email protected] > > *Subject:* Re: [sarif] #401: Improved design of address object design > > After reading the address section changes. I have a couple of comments: > > 1) I would think that a common use of address would be something on the > stack. To facilitate this, the offsetFromParent should be allowed to be > negative as the "bottom" of stack is most logical place to be offset > from, and since some stacks grow from higher to lower memory addresses, > the offset is negative. The use of -1 for unknown mechanism would have > to be changed for this to work. > > 2) Add stack as a kind > > 3) There is a length in physical location, but not all addresses will > appear in a physical location (probably just those without parentIndex) > a length field would be useful. This allows the producers to specify > the lengths of the parent addresses that are regions in memory, and for > consumers to determine them. The length should be allowed to be negative > to handle the stack case cleanly and to indicate that it grows towards > decreasing values (if length is negative the regions is from (address - > offset) to address. For instance if I have an address in a physical > object, a question that I might want to know is does this address > overflow a containing memory region. > > 4) effectiveAddress can be one of two types of addresses: a relative > address to some logical memory/file region, and an absolute memory > address. These concepts are independent, are both useful, and should be > distinguishable from one another. Something like: > > *offsetFromParent* - offset from containing parent (same as existing) > *parentIndex* - direct container address region if there is one (same > as existing) > *offsetFromLogicalAddressRegion* - offset from some base logical region > of memory (like effectiveAddress) > *logicalAddressRegionIndex* - the base logical region of memory the > above offset if relative to > *absoluteMemoryAddress* - absolute memory address of address (physical > or virtual, be consistent). Optional, recommended to only be included > on addresses without parentIndex. > > Right now the effectiveAddress is difficult to use as they can not be > compared without first figuring out what they are relative to and it > isn't clear if the effective address is relative to something or absolute. > > Jim > > On 4/28/19 2:25 PM, Larry Golding (Myriad Consulting Inc) wrote: > > I created and pushed a change draft for Issue #401 > < https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fissues%2F401&data=01%7C01%7Cv-lgold%40microsoft.com%7C97e7012282d14246d4a708d6ccc7981c%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=rQQY6XNIxzEawL4lNiJVQeC8bJ4r%2FCyZE%2FKpe49Uulc%3D&reserved=0 >, > Improved address object design . > > https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ChangeDrafts/Accepted/sarif-v2.0-issue-401-address-rework.docx > < https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fblob%2Fmaster%2FDocuments%2FChangeDrafts%2FAccepted%2Fsarif-v2.0-issue-401-address-rework.docx&data=01%7C01%7Cv-lgold%40microsoft.com%7C97e7012282d14246d4a708d6ccc7981c%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=w24XIuer8elvF%2FGUwVHcrBPlX4iOs0%2FK8dFTR94nk3I%3D&reserved=0 > > > The original design was ambiguous about what address.offset was based > on: the parent object, or address.baseAddress? > > * We renamed offset to offsetFromParent to clear up that ambiguity. > * We removed baseAddress because once it s clear that offset is > based on the parent object, there s no need to duplicate the > information in the child object. > * We added some more kind values ("header", "table") and we (really > Michael) made the definitions of "section" and "segment" more > precise. We (this time, really me ð) also added text acknowledging > that the meanings of these terms are OS-dependent, and recommending > that SARIF producers use the term most appropriate for the target OS. > * Finally, an innovation: we introduced a property effectiveAddress, > which, when not present, can in certain circumstances be calculated > by adding the offsets all the way up the ancestor chain. (The > precise algorithm is in the spec.) This has two benefits: > o It allows the consumer to see the final address without having > to walk all the way up the ancestor chain. This at first /seems/ > duplicative in the same way that baseAddress was (although even > if that were true, effectiveAddress saves the consumer more work > than baseAddress did because baseAddress duplicated information > in a /single/ address object rather than a whole chain of them). > *BUT!* It s not really duplicative, because > o The effectiveAddress property allows a consumer to see the final > address /even if some of the information required to calculate > it is missing from the ancestor addresses/. This allows you to > say, for example, Here is the address of this entry in the > exception handler table; the table entry is a child of the > table, which is itself a child of some other thing without > having to specify the address of the exception handler table if > you don t think that s relevant. > > This draft also includes a small amount of residual editorial feedback. > > As always, the provisional draft and its HTML version are up to date: > > https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ProvisionalDrafts/sarif-v2.0-csd02-provisional.docx > < https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fblob%2Fmaster%2FDocuments%2FProvisionalDrafts%2Fsarif-v2.0-csd02-provisional.docx&data=01%7C01%7Cv-lgold%40microsoft.com%7C97e7012282d14246d4a708d6ccc7981c%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=ABRgRo2Ep2Pq0vvU8w0r9JrgQ4Fg8A%2FXwmx1bHcyf%2BM%3D&reserved=0 > > > https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ProvisionalDrafts/sarif-v2.0-csd02-provisional.htm > < https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foasis-tcs%2Fsarif-spec%2Fblob%2Fmaster%2FDocuments%2FProvisionalDrafts%2Fsarif-v2.0-csd02-provisional.htm&data=01%7C01%7Cv-lgold%40microsoft.com%7C97e7012282d14246d4a708d6ccc7981c%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=vJqNcIBiRtTbPmQsPqNYvt4Xf6sJ9DmGZGM47B765o0%3D&reserved=0 > > > *Please take a look.* We still plan to open the CSD 2 ballot on Monday. > > Thanks, > > Larry >