Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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