1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
|
[[!meta copyright="Copyright © 2011 Free Software Foundation, Inc."]]
[[!meta license="""[[!toggle id="license" text="GFDL 1.2+"]][[!toggleable
id="license" text="Permission is granted to copy, distribute and/or modify this
document under the terms of the GNU Free Documentation License, Version 1.2 or
any later version published by the Free Software Foundation; with no Invariant
Sections, no Front-Cover Texts, and no Back-Cover Texts. A copy of the license
is included in the section entitled [[GNU Free Documentation
License|/fdl]]."]]"""]]
[[!tag open_issue_glibc]]
IRC, freenode, #hurd, 2011-07-22
[additional init-first.c patch]
< tschwinge> civodul: The only thing I wonder about: Roland also once had
done similar changes, which I then found they didn'T work with GCC 4.1,
and backed them out in 08f53ee9d265ffdc7e0affd6acf346cceeb65559 and fixed
the issue differently in d8d27e633a7860b37fd2e3142822b640a066cc0f (and
e17cef66140d4c97710ea88bd8d12612799e1e0f). Have you reviewed this?
< tschwinge> That's in the Savannah glibc repository.
< tschwinge> And this has been in 2007, four years ago. I don't remember
all the details.
< tschwinge> And here is quite a good summary of this stuff, from
init-first.c:
< tschwinge> /* XXX This is all a crock and I am not happy with it.
< tschwinge> This poorly-named function is called by static-start.S,
< civodul> braunr: thanks; i must admit it took me a while to figure it out
;-)
< tschwinge> which should not exist at all. */
< tschwinge> civodul: I can imagine... :-/
< civodul> tschwinge: re Roland's changes, that's weird; i plan to try to
reinstate his change and see if it works
< civodul> now, i won't test with GCC 4.1...
< tschwinge> Yeah...
< tschwinge> I'm happy if it works with 4.4 onwards.
< tschwinge> civodul: And it's safe (in GCC terms) to write to ``* ((void
**) __builtin_frame_address (0) + 1)'', and similar?
< tschwinge> Or should we be coding this few stuff in assembly?
< civodul> tschwinge: well, we should add a compile-time assertion for
__builtin_return_address (0) == *((void**)__builtin_frame_address (0) +
1)
< civodul> (i think GCC can figure it out at compile-time)
< civodul> but on IA32 it should always be true
< civodul> what's the name of glibc's compile-time assert macro already?
< tschwinge> I wonder whether that might interfere with some of GCC's
optimizations?
< civodul> what?
< tschwinge> Well, it seems unclean for me to be modifying a function's
return address from within C code.
< tschwinge> civodul: I added a verify.h in the t/verify.h branch. But
people didn't really like it too much. They rather wanted to directly
inline the array[(cond)?1:-1] code.
< civodul> ok
< civodul> i remember a debate about Gnulib's verify.h
< civodul> i thought something comparable had landed eventually
< tschwinge> civodul: Oh, maybe I missed it.
< tschwinge> civodul: In init-first.c:init, what about the usage of
data[-1] in the else path (not using cthreads) -- is that good as-is?
< civodul> tschwinge: oooh, it probably needs to fixed too
< civodul> but i haven't reached that point yet ;-)
* civodul tries to cross-bootstrap GNU from scratch
< tschwinge> civodul: I'd be happy to learn what was wrong with Roland's
original idea of fixing this. Or perhaps this was a GCC 4.1 bug? Or
perhaps GCC was inlining to much, and then got confused with frames and
return addresses?
< civodul> tschwinge: Roland's change looks good to me, so it could have
been a GCC bug
< civodul> tschwinge: OK to commit the patch to t/init-first.c (with both
data[-1] replaced)?
< tschwinge> civodul: OK, if you are confident that it works with GCC 4.4
onwards. If yes, please add your changelog snippet to .topmsg, and also
add a not that Roland's original code may in fact have been fine, and we
may have hit a compiler bug.
< civodul> tschwinge: OK, will do
< civodul> tschwinge: though regarding Roland's change, i'd prefer to
actually test and see
< tschwinge> civodul: Thanks!
|
