Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#9439 closed bug (fixed)

LlvmCodegen: Overzealous mangler incorrectly transforms user code

Reported by: bgamari Owned by:
Priority: highest Milestone: 7.8.4
Component: Compiler (LLVM) Version: 7.8.2
Keywords: Cc: rwbarton@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking: #9268
Related Tickets: Differential Rev(s): Phab:D150
Wiki Page:

Description

In an attempt to close #4210 the LLVM code generator's mangler was modified in ed67d290e7389bd87a6feea269a0275e0f0f5e2f to rewrite symbol types from @function to @object. This was done in order to prevent the linker from sending reference through the PLT which breaks info tables.

Unfortunately, this mangling was a simple text replacement of @function to @object in the assembler produced by LLVM and made no attempt to distinguish .type directives (which the mangling targets) from other occurrences of the token. As rwbarton unfortunately found out, this means that any occurrences of "@function" in user code (e.g. the LLVM backend itself while compiling GHC) will be rewritten to "@object" in the produced object. Hilarity ensues.

This can be demonstrated by the simple test main = putStrLn "@function", which prints @object in the affected releases (7.8.1 through 7.8.3 thusfar).

Change History (16)

comment:1 Changed 5 years ago by bgamari

Cc: rwbarton@… added
Owner: set to bgamari
Priority: normalhighest

comment:2 Changed 5 years ago by bgamari

Differential Rev(s): D150

comment:3 Changed 5 years ago by rwbarton

Blocking: 9268 added

comment:4 Changed 5 years ago by rwbarton

Differential Rev(s): D150Phab:D150
Milestone: 7.8.4
Owner: changed from bgamari to rwbarton

comment:5 Changed 5 years ago by Reid Barton <rwbarton@…>

In 5895f2b8ffba72a8393e9f712461e6e5ed7ceced/ghc:

LlvmMangler: Be more selective when mangling object types

Summary:
We previously did a wholesale replace of `%function` to `%object` to
mangle object `.type` annotations. This is bad as it can end up
replacing appearances of `"%function"` in the user's code. We now look
for a proper `.type` keyword before performing the replacement.

Thanks to @rwbarton for pointing out the bug.

Test Plan:
Previously,

    $ echo 'main = putStrLn "@function"' > test.hs
    $ ghc -fllvm test.hs
    $ ./test
    @object

Now,
    $ echo 'main = putStrLn "@function"' > test.hs
    $ ghc -fllvm test.hs
    $ ./test
    @function

Reviewers: rwbarton, austin

Reviewed By: rwbarton, austin

Subscribers: phaskell, simonmar, relrod, ezyang, carter

Differential Revision: https://phabricator.haskell.org/D150

GHC Trac Issues: #9439

comment:6 Changed 5 years ago by rwbarton

Owner: rwbarton deleted

comment:7 Changed 5 years ago by rwbarton

Status: newmerge

comment:8 Changed 5 years ago by Austin Seipp <austin@…>

In bbd031134a571c1020945b2548e3fc4795b5047a/ghc:

Bug #9439: Ensure that stage 0 compiler isn't affected

Summary:
Bug #9439 will cause miscompilation of GHC's LLVM backend. Here we
ensure that an affected compiler isn't used to bootstrap.

Test Plan: Attempt to bootstrap GHC with an affected stage 0 compiler.

Reviewers: rwbarton, austin

Reviewed By: austin

Subscribers: simonmar, relrod, ezyang, carter

Differential Revision: https://phabricator.haskell.org/D159

comment:9 Changed 5 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Merged.

comment:10 Changed 5 years ago by Austin Seipp <austin@…>

In 146dd138e2c3b4ec9b211dcbcedf752aeb79d3d1/ghc:

Only test for bug #9439 when llvm is installed

Reviewers: bgamari, austin

Reviewed By: austin

Subscribers: thomie, carter

Differential Revision: https://phabricator.haskell.org/D500

GHC Trac Issues: #9807

comment:11 Changed 5 years ago by rwbarton

Status: closedmerge

As reported by someone on #ghc, the actual fix 5895f2b8ffba72a8393e9f712461e6e5ed7ceced was never merged, only the subsequent bbd031134a571c1020945b2548e3fc4795b5047a.

comment:12 Changed 5 years ago by Austin Seipp <austin@…>

In 1dfab7a8ace5f09f00f8fb695932b4324e88b822/ghc:

Fix detection of llvm-x.x

Summary:
Four bug fixes and a little refactoring.
* `find -perm \mode` should be `find -perm /mode` (#9697)

* `find -regex '$3' should be `find -regex "$3"` (#7661)

From `man sh` on my system (Ubuntu 14.04):
"Enclosing characters in single quotes preserves the literal meaning of all
the characters ..."

* LlcCmd and OptCmd should be passed to ghc, using `-pgmlo` and `-pgmlc`, for
  detection of #9439.

* -pgmlo and -pgmlc were undocumented because of an xml tag misplacement.

Test Plan:
The aclocal.m4 macro has seen about 10 iterations since its inception. Without a
testsuite, I can't guarantee this version is bug free either. It's all pretty
frustrating.

Reviewers: bgamari, austin

Reviewed By: austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D683

GHC Trac Issues: #9697, #7661, #9439

comment:13 Changed 4 years ago by Erik de Castro Lopo <erikd@…>

In 42448e3757f25735a0a5b5e2b7ee456b5e8b0039/ghc:

Do version specific detection of LLVM tools (#10170).

The LLVM developers seem to make breaking changes in the LLVM IR
language between major releases. As a consumer of the LLVM tools
GHC now needs to be locked more tightly to a single version of
the LLVM tools.

GHC HEAD currently only supports LLVM version 3.6. This commit
changes the configure script to look for `llc-3.6` and `opt-3.6`
before looking for `llc` and `opt`. If the former are not found,
but the later are, check that they actually are version 3.6.

At the same time, when detecting known problems with the LLVM
tools (ie #9439) test for it using the versions of the LLVM tools
retrieved from the bootstrap compiler's settings file.

Test Plan: Manual testing.

Reviewers: thomie, rwbarton, nomeata, austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D745

GHC Trac Issues: #10170

comment:14 Changed 4 years ago by bgamari

Status: mergeclosed

7.8 is quite old and this is fixed in 7.10. Closing.

comment:15 Changed 4 years ago by Thomas Miedema <thomasmiedema@…>

In b40e559/ghc:

Build system: simplify *-llvm BuildFlavours (#10223)

Note that SRC_HC_OPTS are added to every Haskell compilation. So
there isn't any need to also add `-fllvm` to GhcStage1HcOpts,
GhcStage2HcOpts and GhcLibHcOpts.

Small bug fix: make sure we test for -fllvm in SRC_HC_OPTS, to check
whether the bootstrap compiler is affected by bug #9439.

Reviewed by: austin

Differential Revision: https://phabricator.haskell.org/D1188

comment:16 Changed 3 years ago by Erik de Castro Lopo <erikd@…>

In 6fe2355/ghc:

configure.ac: Remove checks for bug 9439

Bug #9439 only affects some ghc 7.8 versions of the compiler and since
git HEAD no longer builds with ghc-7.8 we can drop this check.

Test Plan: Works here!

Reviewers: hvr, austin, bgamari

Reviewed By: bgamari

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2427
Note: See TracTickets for help on using tickets.