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

Function merging pass miscompiles identical vararg functions #39692

Closed
vedantk opened this issue Jan 17, 2019 · 4 comments
Closed

Function merging pass miscompiles identical vararg functions #39692

vedantk opened this issue Jan 17, 2019 · 4 comments
Labels
bugzilla Issues migrated from bugzilla ipo Interprocedural optimizations

Comments

@vedantk
Copy link
Collaborator

vedantk commented Jan 17, 2019

Bugzilla Link 40345
Version trunk
OS All
CC @nikic,@vedantk,@whitequark

Extended Description

Example:

#include <stdarg.h>

void escape(int);

void simple_va(const char* fmt, ...)
{
    va_list args;
    va_start(args, fmt);
    int i = va_arg(args, int);
    escape(i);
    va_end(args);
}

void simple_va2(const char* fmt, ...)
{
    va_list args;
    va_start(args, fmt);
    int i = va_arg(args, int);
    escape(i);
    va_end(args);
}

Clang emits (https://godbolt.org/z/C54W53):

; Function Attrs: minsize optsize uwtable
define dso_local void @_Z9simple_vaPKcz(i8* nocapture readnone, ...) local_unnamed_addr #3 {
  tail call void (i8*, ...) @_Z10simple_va2PKcz(i8* nocapture readnone %0) #3
  ret void
}
@vedantk
Copy link
Collaborator Author

vedantk commented Jan 17, 2019

I've disabled merging in this case in r351411.

@nikic
Copy link
Contributor

nikic commented Jan 17, 2019

Might be better to move the check into isThunkProfitable (with an appropriate rename), as emitting an alias should still be fine for this case.

@vedantk
Copy link
Collaborator Author

vedantk commented Jan 17, 2019

Might be better to move the check into isThunkProfitable (with an
appropriate rename), as emitting an alias should still be fine for this case.

Thanks for pointing this out. I've sent out a patch: https://reviews.llvm.org/D56865

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@fhahn
Copy link
Contributor

fhahn commented Apr 11, 2022

IIUC this has been fixed in b537b94. Closing the issue, but please reopen the issue if this is incorrect.

@fhahn fhahn closed this as completed Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla ipo Interprocedural optimizations
Projects
None yet
Development

No branches or pull requests

3 participants