This is an archive of the discontinued LLVM Phabricator instance.

Clang, increase upper bound of partially initialized array sizes
Needs ReviewPublic

Authored by OfekShochat on Sep 10 2022, 12:55 PM.

Details

Reviewers
klimek
efriedma
Summary

fixes issue with emitting partially initialized constant arrays larger than 2^32.
issue #57353 on github.

Diff Detail

Event Timeline

OfekShochat created this revision.Sep 10 2022, 12:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 10 2022, 12:55 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
OfekShochat requested review of this revision.Sep 10 2022, 12:55 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 10 2022, 12:55 PM
OfekShochat retitled this revision from [Clang] changing behavior of constant array emission to Clang, increase upper bound of partially initialized array sizes.Sep 10 2022, 12:56 PM
shafik added a subscriber: shafik.Sep 12 2022, 1:29 PM
shafik added inline comments.
clang/lib/Sema/SemaInit.cpp
866

setElementIndex(...) takes unsigned as well and therefore InitializedEntity also uses unsigned.

I briefly looked at this with another bug https://github.com/llvm/llvm-project/issues/57317

and I believe the 32 bit assumption is made in a lot of this code in this area.

efriedma added inline comments.Sep 12 2022, 2:13 PM
clang/lib/Sema/SemaInit.cpp
866

You don't need to solve everything at once...

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
3167

Can you split this into a separate patch (with an appropriate testcase)?

Clang, increase upper bound of partially initialized array sizes

fixes issue with emitting partially initialized constant arrays larger than 2^32.
issue #57353 on github.

OfekShochat marked an inline comment as done.Sep 14 2022, 2:22 AM

I created a diff for the AsmPrinter part, its at D133845

clang/lib/Sema/SemaInit.cpp
866

oh, right, should I change that after I split the diff in AsmPrinter to another patch?

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
3167

yep

OfekShochat marked an inline comment as done.Sep 25 2022, 3:07 AM

@efriedma would you be able to commit this?