[all-commits] [llvm/llvm-project] 93764f: [modules] Fix miscompilation when using two Record...

[Volodymyr Sapsai via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 93764ff6e2005b92057cffa9b3866307e2de260f
      https://github.com/llvm/llvm-project/commit/93764ff6e2005b92057cffa9b3866307e2de260f
  Author: Volodymyr Sapsai <vsapsai at apple.com>
  Date:   2021-08-30 (Mon, 30 Aug 2021)

  Changed paths:
    M clang/include/clang/Serialization/ASTReader.h
    M clang/lib/Serialization/ASTCommon.cpp
    M clang/lib/Serialization/ASTReaderDecl.cpp
    A clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Headers/RecordDef.h
    A clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Modules/module.modulemap
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Headers/RecordDefCopy.h
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Modules/module.modulemap
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Hidden.h
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Visible.h
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Modules/module.modulemap
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h
    A clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap
    A clang/test/Modules/merge-record-definition-nonmodular.m
    A clang/test/Modules/merge-record-definition-visibility.m
    A clang/test/Modules/merge-record-definition.m

  Log Message:
  -----------
  [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

When deserializing a RecordDecl we don't enforce that redeclaration
chain contains only a single definition. So if the canonical decl is not
a definition itself, `RecordType::getDecl` can return different objects
before and after an include. It means we can build CGRecordLayout for
one RecordDecl with its set of FieldDecl but try to use it with
FieldDecl belonging to a different RecordDecl. With assertions enabled
it results in

> Assertion failed: (FieldInfo.count(FD) && "Invalid field for record!"),
> function getLLVMFieldNo, file llvm-project/clang/lib/CodeGen/CGRecordLayout.h, line 199.

and with assertions disabled a bunch of fields are treated as their
memory is located at offset 0.

Fix by keeping the first encountered RecordDecl definition and marking
the subsequent ones as non-definitions. Also need to merge FieldDecl
properly, so that `getPrimaryMergedDecl` works correctly and during name
lookup we don't treat fields from same-name RecordDecl as ambiguous.

rdar://80184238

Differential Revision: https://reviews.llvm.org/D106994