[llvm] [yaml2obj] Don't use uninitialized Type (PR #123274)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 17 00:50:18 PST 2025
vitalybuka wrote:
> I think we may have been looking at different cases, though I'm not 100% certain. When I tried this out yesterday, I omitted the `Type` field from the YAML entirely. As a result, the `getStringValue` call on line 1601 resulted in an error (because `Type` was missing) and `TypeStr` was left as an empty string. Subsequently, `Type` was then read without being initialised.
>
> Looking at the code and your description, in the case you've been looking at, I'm guessing you might have a `Type` field in the YAML that starts with `SHT_` but which doesn't have a valid mapping (based on the test you described, because the machine-specific section type is used with a non-applicable machine). Is that correct?
>
Close, there are SHT_s which are ON/OF deppending on machine type set.
But the same happens with non-existent SHT_s as well.
> Your fix only fixes the latter case, which is insufficient. The correct fix takes place outside the if statement at line 1602, in one form or another. If, as you described, it's not possible to simply do `return` from this function after seeing the invalid/missing `Type` value in the YAML, then we should adopt MaskRay's suggestion of zero-initialising the field in the first place, with an `SHT_NULL` value like you are doing already (but at the declaration point, not here). state anyway.
To clarify "zero-initialising the field", are we talking about local variable 'Type', not a field
> This might be preferable to checking the error.
EC contains some error, not sure what we can do here?
>
> What do you think?
Yes, that essentially what was https://github.com/llvm/llvm-project/pull/123137 , I just copied wrong 0 constant ET_NONE there.
https://github.com/llvm/llvm-project/pull/123274
More information about the llvm-commits
mailing list