[PATCH] D76594: [clang][AST] User relative offsets inside AST file

Dmitry Polukhin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 23 12:00:57 PDT 2020


DmitryPolukhin added a comment.

In D76594#1937289 <https://reviews.llvm.org/D76594#1937289>, @sammccall wrote:

> This looks reasonable to me, though I'm not familiar enough with the code to do a good review.
>
> Just wanted to confirm my understanding: the problematic offsets that are now section-relative instead of file-relative are only used in two sections of the file (macro directives and sloc entries). These sections seem likely to be small relative to others that hold e.g. the AST. So this should raise the effective size limit by a lot, more likely 100x than 5?


Yes, problematic offsets are now section relative and the sections several times smaller than the whole file. In suspect in some artificial cases these section can be pretty large. For example, you can create source file with only macro and you can exceed limit for on macro size alone. Moreover I'm not sure that files bigger than 4G can be processed correctly. Because there are other places where 32 bit numbers are used for some IDs or byte offsets.

> (We've coincidentally started to see large crashing preambles this week, so thanks for this! I think we probably don't need to add any detection/recovery if this limit is high enough)

I added asserts for all new places where overflow can happen but perhaps we also need something for release builds. If you have cases when you reach size limit, it would be very helpful if you can try this patch on these cases. I tested it on couple ~700M files but my examples relatively unified and follow the same pattern.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76594/new/

https://reviews.llvm.org/D76594





More information about the cfe-commits mailing list