[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150821142336.GA32742@brightrain.aerifal.cx>
Date: Fri, 21 Aug 2015 10:23:36 -0400
From: Rich Felker <dalias@...c.org>
To: Christian Lamparter <chunkeey@...glemail.com>
Cc: musl@...ts.openwall.com, Bobby Bingham <koorogi@...rogi.info>,
openwrt-devel@...ts.openwrt.org
Subject: Re: SuperH conflict of arch/sh/__set_thread_area vs
thread/__set_thread_area
On Fri, Aug 21, 2015 at 04:07:39PM +0200, Christian Lamparter wrote:
> (Added Openwrt-dev - since this isn't a musl issue)
>
> On Thursday, August 20, 2015 12:21:55 PM Christian Lamparter wrote:
> > On Wednesday, August 19, 2015 11:04:02 PM Rich Felker wrote:
> > > On Thu, Aug 20, 2015 at 02:44:11AM +0200, Christian Lamparter wrote:
> > > > I'm trying to add a port for a SH4-like ARCH to OpenWRT, which uses the latest
> > > > musl-1.1.10 as the default libc. I'm having the following problem when building
> > > > the toolchain:
> > > >
> > > > During the final linker-step, the symbol "__set_thread_area" declared twice.
> > > > This is because the SH architecture provides a separate __set_thread_area [0],
> > > > (other archs use the standard syscall wrapper from [1]).
> > > >
> > > > Obviously, I want this issue fixed. However I'm new to SuperH and musl, that's
> > > > why I need advise :-D. For now, I defined the src/thread/__set_thread_area as
> > > > a weak symbol. Now, that's just a crude hack, what would be better solution?
> > > > (I can make and post the patch if necessary - But sadly, I can't test it on the
> > > > hardware yet)?
> > >
> > > Bobby Bingham's reply explains what the issue is. Did you make a new
> > > arch name rather than using the existing sh arch for your port?
> > Initially, yes I did. I had the ARCH at "sh4". This was because OpenWRT
> > already had infrastructure for some sh-(sub)targets (sh3, sh3eb, sh4, sh4eb)
> > in place. But they seem to be unused and untested. The only target which has
> > support for SuperH is UserModeLinux. However, it will probably run into the
> > same issue.
> >
> > Now, I've changed ARCH to "sh" and set the CPU_TYPE to sh4 [toolchain
> > dir changed to toolchain-sh_sh4_gcc-5.2.0_musl-1.1.10]. But still no
> > luck, the original error code remains the same.
> >
> > src/thread/__set_thread_area.lo: In function `__set_thread_area':
> > __set_thread_area.c:(.text+0x0): multiple definition of `__set_thread_area'
> > arch/sh/src/__set_thread_area.lo:__set_thread_area.c:(.text+0x0): first defined here
> > collect2: error: ld returned 1 exit status
> > Makefile:142: recipe for target 'lib/libc.so' failed
>
> Ok, I have a update and I fixed this issue with __set_thread_area.s.
> The reason why this didn't work and was harder to debug than usual is
> because of the involvement of the "patch" utility with the -E flag
> (--remove-empty-files).
>
> Openwrt adds a number of patches to the musl-1.1.10.tar.gz [0] (actually
> they seem to be cherry-picked from musl git). Now, one of the patches
> "001-git-2015-07-22.patch" contains:
>
> commit f9d84554bae0fa17c9a1d724549c4408022228a5 [1]
> Author: Rich Felker <dalias@...ifal.cx>
> Date: Tue Jun 16 14:28:30 2015 +0000
>
> add support for sh2 interrupt-masking-based atomics to sh port
>
> This commit reduces the "src/thread/sh/__set_thread_area.s" to an empty
> file (but it doesn't remove it). But due to bad luck, the script that
> patches the file [2] (it's called patch-kernel.sh, but it's used to patch
> the toolchain as well) uses the pesky "-E" flag (line 40):
> [...] ${PATCH:-patch} -f -p1 -E -d ${targetdir}
> ^^
>
> ==> And that's why the __set_thread_area.s file disappears.
>
> I came up with two possible fixes for OpenWRT, unless you want to
> fix this in musl (via non-empty __set_thread_area) instead:
There's a third possible fix: Just waiting for that patch to be
dropped once they're using a base release of musl that already has it
upstream. However I suspect this is an issue that could come up again,
since IIRC musl has a number of empty files in the source tree and we
sometimes add more, so my leaning would be to get OpenWRT to fix it.
> 1. have some content in __set_thread_area.s to prevent it from being
> removed.
>
> ---
>
> --- a/src/thread/sh/__set_thread_area.s
> +++ b/src/thread/sh/__set_thread_area.s
> @@ -0,0 +1,1 @@
> +/* the patch utility might remove empty files */
>
> ---
This seems like a non-solution since the problem is not the code
that's in musl (that would be fine) but a patch OpenWRT is shipping
based on commit to musl. The latter patch is what would need to be
fixed, and it could be fixed manually if this approach is desired, but
I think that's a bad idea because the issue might come up again in the
future with other files.
> 2. patch patch-kernel.sh
>
> ---
> --- a/scripts/patch-kernel.sh
> +++ b/scripts/patch-kernel.sh
> @@ -37,7 +37,7 @@ for i in ${patchdir}/${patchpattern} ; do
> [ -d "${i}" ] && echo "Ignoring subdirectory ${i}" && continue
> echo ""
> echo "Applying ${i} using ${type}: "
> - ${uncomp} ${i} | ${PATCH:-patch} -f -p1 -E -d ${targetdir}
> + ${uncomp} ${i} | ${PATCH:-patch} -f -p1 -d ${targetdir}
> if [ $? != 0 ] ; then
> echo "Patch failed! Please fix $i!"
> exit 1
>
> ---
>
> Does anyone have any preferences? I'm opting for "patch patch-kernel"
> (and I will send a proper patch next week to OpenWRT even if no one
> cares now :-D )
This seems like a better approach to me, but I don't know if it would
adversely affect the kernel or other components.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
