• Igor Sheludko's avatar
    Reland^2 "[csa] Fix semantics of PopAndReturn" · d2ab873d
    Igor Sheludko authored
    This is a reland of 3593ee83
    
    The MSAN doesn't seem to be considering initializing stores via inline
    assembly as such (in a new cctest helper GetStackPointer()), so this
    reland attempt fixes the issue and ensures that the MSAN bot is happy.
    
    Original change's description:
    > Reland "[csa] Fix semantics of PopAndReturn"
    >
    > This is a reland of 5e5eaf79
    >
    > This CL fixes the "function returns address of local variable" issue
    > which GCC was complaining about by using inline assembly instead of
    > address of a local for getting stack pointer approximation.
    >
    > Original change's description:
    > > [csa] Fix semantics of PopAndReturn
    > >
    > > This CL prohibits using PopAndReturn from the builtins that
    > > have calling convention with arguments on the stack.
    > >
    > > This CL also updates the PopAndReturn tests so that even off-by-one
    > > errors in the number of poped arguments are caught which was not the
    > > case before.
    > >
    > > Motivation:
    > >
    > > PopAndReturn is supposed to be using ONLY in CSA/Torque builtins for
    > > dropping ALL JS arguments that are currently located on the stack.
    > > Disallowing PopAndReturn in builtins with stack arguments simplifies
    > > semantics of this instruction because in case of presence of declared
    > > stack parameters it's impossible to distinguish the following cases:
    > > 1) stack parameter is included in JS arguments (and therefore it will
    > >    be dropped as a part of 'pop' number of arguments),
    > > 2) stack parameter is NOT included in JS arguments (and therefore it
    > >    should be dropped in ADDITION to the 'pop' number of arguments).
    > >
    > > This issue wasn't noticed before because builtins with stack parameters
    > > relied on adapter frames machinery to ensure that the expected
    > > parameters are present on the stack, but on the same time the adapter
    > > frame tearing down code was effectively recovering the stack pointer
    > > potentially broken by the CSA builtin.
    > >
    > > Once we get rid of the arguments adapter frames keeping stack pointer
    > > in a valid state becomes crucial.
    > >
    > > Bug: v8:5269, v8:10201
    > > Change-Id: Id3ea9730bb0d41d17999c73136c4dfada374a822
    > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2460819
    > > Commit-Queue: Igor Sheludko <ishell@chromium.org>
    > > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
    > > Cr-Commit-Position: refs/heads/master@{#70454}
    >
    > Tbr: tebbi@chromium.org
    > Bug: v8:5269
    > Bug: v8:10201
    > Change-Id: Ic1a05fcc4efd2068538bff28189545cfd2617d9b
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2465839
    > Reviewed-by: Igor Sheludko <ishell@chromium.org>
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Igor Sheludko <ishell@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#70483}
    
    Tbr: tebbi@chromium.org
    Cq-Include-Trybots: luci.v8.try:v8_linux64_msan_rel_ng
    Bug: v8:5269
    Bug: v8:10201
    Change-Id: Ib09af2d1260bb42ac26aabface14e6b83b3efec4
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2467847
    Commit-Queue: Igor Sheludko <ishell@chromium.org>
    Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
    Reviewed-by: 's avatarVictor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70492}
    d2ab873d
test-code-stub-assembler.cc 151 KB