Skip to content

Conversation

rastogishubham
Copy link

This commit has 3 minor, but important fixes

  1. Ensure that MCLEBFragmentRef::create uses the passed-in FragmentsContents to create the cas object.

  2. Ensure that MCCVInlineLineTableFragmentRef::create uses the passed-in FragmentsContents to create the cas object.

  3. Make FT_CVDefRange a mergeable fragment, as all other non-encoded fragments are mergeable.

rdar://159458401

rdar://159216364

This commit has 3 minor, but important fixes

1. Ensure that MCLEBFragmentRef::create uses the passed-in
FragmentsContents to create the cas object.

2. Ensure that MCCVInlineLineTableFragmentRef::create uses the passed-in
FragmentsContents to create the cas object.

3. Make FT_CVDefRange a mergeable fragment, as all other non-encoded
fragments are mergeable.

rdar://159458401

rdar://159216364
@rastogishubham
Copy link
Author

@swift-ci please test llvm

Copy link

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

I can see LEB is covered by existing test case. Can you add test for other changes?

@rastogishubham
Copy link
Author

I can see LEB is covered by existing test case. Can you add test for other changes?

I don't know how to create a testcase for MCCVInlineLineTableFragmentRef, how do I write an .ll file that will guarantee that the MCAssembler will produce a MCCVInlineLineTableFragment with both contents and variable contents?

As for making FT_CVDefRange a mergeable fragment, it is not a bug fix, it is just an NFC change for consistency

@cachemeifyoucan
Copy link

I don't know how to create a testcase for MCCVInlineLineTableFragmentRef, how do I write an .ll file that will guarantee that the MCAssembler will produce a MCCVInlineLineTableFragment with both contents and variable contents?

You want to find that out. We need to gradually increase the coverage of MCCAS in lit testing.

@ojhunt
Copy link

ojhunt commented Sep 3, 2025

I don't know how to create a testcase for MCCVInlineLineTableFragmentRef, how do I write an .ll file that will guarantee that the MCAssembler will produce a MCCVInlineLineTableFragment with both contents and variable contents?

You want to find that out. We need to gradually increase the coverage of MCCAS in lit testing.

This is breaking existing tests for me, so it would seem reasonable to land the fix as it stands while working out how to add additional tests, or I guess to revert the original PR :-/

@rastogishubham
Copy link
Author

@cachemeifyoucan the only way to figure it out would be to get an MCCAS round-trip verification error where a CVInlineLineTableFragment contains both, variable content, and fixed content, and ideally a relocation addend too. I don't think anyone can write an ll file by hand to make that happen

@rastogishubham rastogishubham merged commit ea351fe into next Sep 4, 2025
0 of 2 checks passed
@rastogishubham rastogishubham deleted the MCCASNextFix branch September 4, 2025 19:48
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.

3 participants