Skip to content

Comments

Adds a new C++ test, with associated known_bugs entry for current failure#265

Open
kquick wants to merge 1 commit intomasterfrom
add_p2_disasm_test
Open

Adds a new C++ test, with associated known_bugs entry for current failure#265
kquick wants to merge 1 commit intomasterfrom
add_p2_disasm_test

Conversation

@kquick
Copy link
Member

@kquick kquick commented Jan 2, 2024

[Requires #264 to be actually used.]

@@ -0,0 +1,10 @@
##> rootMatchName: p2.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a corresponding issue number for this bug? Should we make one?

Using this module's llvm-disasm to convert the clang++ generated bitcode into assembly (.ll), the resulting assembly is invalid for llvm-as:

error: expected type
define linkonce_ddr default i32 @_ZN4uorb14uorb_advertiseEPK21orb_metadata(%class.uorb* %32

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is just a typo, it should be linkonce_odr. The data member is named correctly, the error is here: https://github.com/elliottt/llvm-pretty/blob/8124fc0265b6ca5fde5812c789f40b2ea54af678/src/Text/LLVM/PP.hs#L463

With this fixed, there is an instance of #222 which is probably the same cause.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reduced p2.cpp (for #222) to

struct h {
  bool a;
};

void g(struct h *s) {
}

void f() {
  struct h s;
}

I do not have a fix yet, but the bitcode is now small enough to walk through manually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great sleuthing!

I actually think the culprit here isn't #222, but rather #260, as the diff failure is caused by improper inlining. I'll add this example to the comments in #260.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants