Opened 4 years ago

Closed 3 years ago

#8951 closed bug (fixed)

genSym uses atomic_inc but doesn't link arm_atomic_spin_lock

Reported by: cjwatson Owned by:
Priority: normal Milestone: 7.8.4
Component: Build System Version: 7.8.1-rc1
Keywords: Cc: bgamari
Operating System: Linux Architecture: arm
Type of failure: Building GHC failed Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

On Debian armel, a snapshot of GHC 7.8 from 20140228 fails to build as follows:

https://buildd.debian.org/status/fetch.php?pkg=ghc&arch=armel&ver=7.8.20140228-1&stamp=1394495564

"inplace/bin/ghc-stage2" -o utils/haddock/dist/build/tmp/haddock -hisuf hi -osuf  o -hcsuf hc -static  -H32m -O -lffi -optl-pthread -optc-mlong-calls    -hide-all-packages -i -iutils/haddock/driver -iutils/haddock/src -iutils/haddock/vendor/attoparsec-0.10.4.0 -iutils/haddock/dist/build -iutils/haddock/dist/build/autogen -Iutils/haddock/dist/build -Iutils/haddock/dist/build/autogen    -optP-DIN_GHC_TREE -optP-include -optPutils/haddock/dist/build/autogen/cabal_macros.h -package Cabal-1.18.1.3 -package array-0.5.0.0 -package base-4.7.0.0 -package bytestring-0.10.4.0 -package containers-0.5.4.0 -package deepseq-1.3.0.2 -package directory-1.2.0.2 -package filepath-1.3.0.2 -package ghc-7.8.0.20140228 -package xhtml-3000.2.1 -funbox-strict-fields -Wall -fwarn-tabs -O2 -XHaskell2010  -no-user-package-db -rtsopts      -odir utils/haddock/dist/build -hidir utils/haddock/dist/build -stubdir utils/haddock/dist/build     utils/haddock/dist/build/Main.o utils/haddock/dist/build/Documentation/Haddock.o utils/haddock/dist/build/Data/Attoparsec.o utils/haddock/dist/build/Data/Attoparsec/ByteString.o utils/haddock/dist/build/Data/Attoparsec/ByteString/Char8.o utils/haddock/dist/build/Data/Attoparsec/Combinator.o utils/haddock/dist/build/Data/Attoparsec/Number.o utils/haddock/dist/build/Data/Attoparsec/ByteString/FastSet.o utils/haddock/dist/build/Data/Attoparsec/ByteString/Internal.o utils/haddock/dist/build/Data/Attoparsec/Internal.o utils/haddock/dist/build/Data/Attoparsec/Internal/Types.o utils/haddock/dist/build/Haddock.o utils/haddock/dist/build/Haddock/Interface.o utils/haddock/dist/build/Haddock/Interface/Rename.o utils/haddock/dist/build/Haddock/Interface/Create.o utils/haddock/dist/build/Haddock/Interface/AttachInstances.o utils/haddock/dist/build/Haddock/Interface/LexParseRn.o utils/haddock/dist/build/Haddock/Interface/ParseModuleHeader.o utils/haddock/dist/build/Haddock/Parser.o utils/haddock/dist/build/Haddock/Parser/Util.o utils/haddock/dist/build/Haddock/Utf8.o utils/haddock/dist/build/Haddock/Utils.o utils/haddock/dist/build/Haddock/Backends/Xhtml.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Decl.o utils/haddock/dist/build/Haddock/Backends/Xhtml/DocMarkup.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Layout.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Names.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Themes.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Types.o utils/haddock/dist/build/Haddock/Backends/Xhtml/Utils.o utils/haddock/dist/build/Haddock/Backends/LaTeX.o utils/haddock/dist/build/Haddock/Backends/HaddockDB.o utils/haddock/dist/build/Haddock/Backends/Hoogle.o utils/haddock/dist/build/Haddock/ModuleTree.o utils/haddock/dist/build/Haddock/Types.o utils/haddock/dist/build/Haddock/Doc.o utils/haddock/dist/build/Haddock/Version.o utils/haddock/dist/build/Haddock/InterfaceFile.o utils/haddock/dist/build/Haddock/Options.o utils/haddock/dist/build/Haddock/GhcUtils.o utils/haddock/dist/build/Haddock/Convert.o utils/haddock/dist/build/Paths_haddock.o    
/«PKGBUILDDIR»/compiler/stage2/build/libHSghc-7.8.0.20140228.a(genSym.o): In function `genSym':
genSym.c:(.text+0x84): undefined reference to `arm_atomic_spin_lock'
genSym.c:(.text+0x88): undefined reference to `arm_atomic_spin_unlock'
collect2: error: ld returned 1 exit status
make[2]: *** [utils/haddock/dist/build/tmp/haddock] Error 1

Our armel architecture satisfies "arm_HOST_ARCH && defined(arm_HOST_ARCH_PRE_ARMv6)", so cas() in includes/stg/SMP.h will (uniquely among architectures) not be entirely inline: it calls arm_atomic_spin_lock and arm_atomic_spin_unlock, which are in libHSrts. 7.8 introduces compiler/cbits/genSym.c which uses atomic_inc. How should the linkage requirements be satisfied here?

Change History (10)

comment:1 Changed 4 years ago by nomeata

Milestone: 7.8.1

Setting a milestone to make sure this is not lost. If this was presumptuous, please undo, or maybe change to 7.8.2.

comment:2 Changed 3 years ago by thoughtpolice

Milestone: 7.8.37.8.4

Moving to 7.8.4.

comment:3 Changed 3 years ago by thoughtpolice

Milestone: 7.8.47.10.1

Moving (in bulk) to 7.10.4

comment:4 Changed 3 years ago by nomeata

Milestone: 7.10.17.8.4

Moving milestone back to 7.8 branch, this is worth a stable update, if a fix is available.

comment:5 Changed 3 years ago by bgamari

Cc: bgamari added

comment:6 Changed 3 years ago by bgamari

Hmm, so the symbols in question are defined in rts/OldARMAtomic.c. These are necessary as this build occurred on an ARMv5 box. It appears OldARMAtomic.o is being compiled into libRTS so this raises the question of why it's not being found by the linker.

nomeata, however, is doing his builds on ARMv7 hardware. In light of this it's likely that there are two bugs here:

  1. nomeata's build is using the pre-ARMv6 fallbacks unnecessarily
  2. the pre-ARMv6 fallbacks aren't compiled/linked properly

comment:7 Changed 3 years ago by nomeata

More info:

genSym.c is linked into libghcHS. It contains code that depends on THREADED_RTS, but that’s a bit fishy: There is only one libghcHS that needs to work with both threaded and non-threaded RTSs.

This code in compiler/ghc.mk might be related:

ifeq "$(GhcThreaded)" "YES"
# We pass THREADED_RTS to the stage2 C files so that cbits/genSym.c will bring
# the threaded version of atomic_inc() into scope.
compiler_stage2_CONFIGURE_OPTS += --ghc-option=-optc-DTHREADED_RTS
endif

So one thing I’m trying is to provide the stubs from rts/OldARMAtomic.c even in the unthreaded RTS. It will be dead code, but at least the linker will be happy.

comment:8 Changed 3 years ago by nomeata

Status: newpatch

So one thing I’m trying is to provide the stubs from rts/OldARMAtomic.c even in the unthreaded RTS. It will be dead code, but at least the linker will be happy.

This helped, and seems to be the right thing to do. Patch at Phab:D564. I’m not committing it myself, as this is not my area of expertise...

It would also be nice to get this into 7.8.4, as the Debian package will have to carry that anyways....

comment:9 Changed 3 years ago by Austin Seipp <austin@…>

In df1307f079ae69dcd735e0973de987b141d509da/ghc:

Link pre-ARMv6 spinlocks into all RTS variants

Summary:
For compatibility with ARM machines from pre v6, the RTS provides
implementations of certain atomic operations. Previously, these
were only included in the threaded RTS.

But ghc (the library) contains the code in compiler/cbits/genSym.c, which
uses these operations if there is more than one capability. But there is only
one libHSghc, so the linker wants to resolve these symbols in every case.

By providing these operations in all RTSs, the linker is happy. The only
downside is a small amount of dead code in the non-threaded RTS on old ARM
machines.

Test Plan: It helped here.

Reviewers: bgamari, austin

Reviewed By: bgamari, austin

Subscribers: carter, thomie

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

GHC Trac Issues: #8951

comment:10 Changed 3 years ago by thoughtpolice

Resolution: fixed
Status: patchclosed

Merged to 7.8.4.

Note: See TracTickets for help on using tickets.