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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to