Re: hurd: PIE support

[Simon Marchi]

On 2017-12-27 10:44 AM, Samuel Thibault wrote:
> Hello,
> 
> Simon Marchi, on mar. 26 déc. 2017 21:48:23 -0500, wrote:
>> We would also need a ChangeLog entry.
> 
> Well, I had put a ChangeLog entry at the top of the posted file. Is
> something else needed?  In the patch I have just sent to the list I have
> directly put the gdb/ChangeLog update in addition to the commit message
> log.  If that is what is wanted, then the gdb/CONTRIBUTE file should be
> update, otherwise frustration happens on both sides.
> 
> Samuel
> 

Ah, sorry I missed it.  I ended up using git-apply to apply the patch (git-am
wouldn't work), which looses the information before the diff.

I'll reply to the other mail.

Simon

Re: Rust support

[Joshua Branson]

Awesome!!! Keep up the cool work!

On Mon, Dec 25, 2017, at 8:19 PM, David Michael wrote:
> Hi,
> 
> I've been poking at cross-compiling Rust projects for Hurd on and off
> for a couple of weeks, and I thought I'd upload what I have now for
> comments.  A new snapshot of my silly Hurd distro is available at
> https://github.com/dm0-/gnuxc updated with the Rust reimplementation
> of librsvg, so there is an example user of this.
> 
> The distro commit contains a patch[1] against the current Rust release
> (1.22.1) which can be used to build std libraries for Hurd with your
> Linux-based system's existing rustc.  I've also pushed the patch
> against master[2].  See the RPM spec file[3] for the commands to build
> the std libraries and a target spec JSON file.
> 
> Since Rust doesn't have a default path, the environment variable
> RUST_TARGET_PATH must be set to the directory containing the JSON spec
> file.  With that and the rlib files installed to
> /usr/lib/rustlib/i686-pc-gnu/lib, Rust projects can be built for Hurd
> with "cargo --target=i686-pc-gnu build".  The command "rustc
> --codegen=ar=i686-pc-gnu-gcc-ar --codegen=linker=i686-pc-gnu-gcc
> --target=i686-pc-gnu" will directly compile Rust files without cargo.
> 
> Note that projects will likely depend on libc, which will need to be
> patched with the same changes used to build std.  The librsvg
> package[4] has an example of how to do this with vendored crates, but
> other projects may require you to set up a local registry and patch it
> there.
> 
> The current implementation reuses the Linux code with some additional
> conditional blocks, but I think it may be better to start a new OS
> port for Hurd.  Otherwise, it may turn into whack-a-mole, tweaking
> constants when new packages encounter bugs, or adding more conditions
> as the Linux implementation is developed.  I'll continue to work on
> this (at an as yet undetermined pace) to hopefully make it
> upstreamable, but any help or feedback from the Hurd side would be
> appreciated to prevent me from doing something dumb.  In case anyone
> else does want to work on this before I get to it, I'd suggest
> prioritizing porting the libc crate--the sooner it has a release with
> Hurd support, the sooner every other project depending on a specific
> libc version will be able to update to build out of the box.
> 
> Thanks.
> 
> David
> 
> [1] 
> https://github.com/dm0-/gnuxc/blob/master/patches/rust-1.22.1-hurd-port.patch
> [2] https://github.com/dm0-/rust/commits/hurd
> [3] https://github.com/dm0-/gnuxc/blob/master/specs/gnuxc-rust.spec#L39
> [4] https://github.com/dm0-/gnuxc/blob/master/make.pkg.d/librsvg.mk#L8
> 

Re: Bug#881569: [Fwd: gdb: FTBFS on hurd-i386]

[Svante Signell]

On Tue, 2017-12-26 at 09:40 +0100, Héctor Orón Martínez wrote:
> Hello Svante,
> 
> On Sat, Dec 23, 2017 at 7:41 PM, Svante Signell
>  wrote:
> > Hello,
> > 
> > These patches was submitted to Debian November 13 2017. Nothing has
> > happened so far, so maybe upstream would be interested to consider
> > the
> > patches for next release.
> 
> I would like to have them applied in Debian package for next release,
> however it is taking more time than I thought, then I am going on
> vacations. If you are able to upload to Debian, feel free to NMU the
> package, otherwise, I'll try to have a look in a couple weeks.

Hi again,

Unfortunately I'm not authorized to NMU that package. However, the
reason for submitting the patches upstream is that they should be
applied there. In the meanwhile we could create a Debian package of
8.0.1 (or later). If you don't have time maybe Samuel could help out
here?

Thanks for your reply anyway!

Re: race condition destroying condition variables

[Samuel Thibault]

Brent W. Baccala, on mer. 27 déc. 2017 15:07:51 -0500, wrote:
> On Wed, Dec 27, 2017 at 2:31 PM, Samuel Thibault <[1]samuel.thiba...@gnu.org>
> wrote:
> 
> Ok, so even if Posix explicitly says that it has undefined behavior,
> since nptl behaves fine we should probably behave fine too.
> 
> My point was that since Linux waits for the other threads in
> pthread_cond_destroy(), we should too. 

Well, that's exactly what I wrote :)

Samuel

Re: race condition destroying condition variables

[Brent W. Baccala]

On Wed, Dec 27, 2017 at 2:31 PM, Samuel Thibault 
wrote:

> Hello,
>
> Brent W. Baccala, on mar. 26 déc. 2017 23:06:13 -0500, wrote:
> > Also, the Linux source code in nptl/ includes the following comment:
> >
> >   /* If there are waiters which have been already signalled or
> >  broadcasted, but still are using the pthread_cond_t structure,
> >  pthread_cond_destroy needs to wait for them.  */
>
> Ok, so even if Posix explicitly says that it has undefined behavior,
> since nptl behaves fine we should probably behave fine too.
>

Let me clarify - that comment precedes a block of code in
pthread_cond_destroy() that waits for the other threads.

See glibc-2.23/nptl/pthread_cond_destroy.c, around line 50.

That code's been rewritten in glibc-2.25, but the requirement is still
there.

glibc-2.25/nptl/pthread_cond_destroy.c, lines 38-40:

Thus, we can assume that all waiters that are still accessing the condvar
> have been woken.  We wait until they have confirmed to have woken up by
> decrementing __wrefs.


... and then it waits for "wrefs >> 3" to become zero.

My point was that since Linux waits for the other threads in
pthread_cond_destroy(), we should too.

agape
brent

Re: race condition destroying condition variables

[Samuel Thibault]

Hello,

Brent W. Baccala, on mar. 26 déc. 2017 23:06:13 -0500, wrote:
> Also, the Linux source code in nptl/ includes the following comment:
> 
>   /* If there are waiters which have been already signalled or
>      broadcasted, but still are using the pthread_cond_t structure,
>      pthread_cond_destroy needs to wait for them.  */

Ok, so even if Posix explicitly says that it has undefined behavior,
since nptl behaves fine we should probably behave fine too.

> The problem that I've got now is that I've changed the size of the condition
> variables by adding an extra field,

Not a good idea, for the resaons you mention :)

> First, I'd like to build the packages using the same scripts used to build the
> Debian distribution packages.  Is this available somewhere?

Just run dpkg-buildpackage from the package source, or use pdebuild, or
use a wanna-build infrastructure. Nothing simple, at any rate.

> Next, how will modifying the memory layout of condition variables affect our
> packaging?  I figure that we'll have to bump libc0.3 to libc0.4, and that
> should pretty much do the trick, right?

Yes, but that's a *HUGE* transition, which is painful. Better use symbol
versioning, so that old non-recompiled programs use the old
implementation, and recompiled programs used the new.

> Finally, I'm wondering if we need to change this at all.  The "__data" field 
> in
> struct __pthread_cond seems unused, at least in the libpthreads directory.  Is
> there any use for this pointer?  Or can I use those bits?

AFAICT it was used for compatibility with libcthreads, which we don't
care any more, so I'd say feel free to use it, and avoid the transition
altogether.

Samuel

Re: hurd: PIE support

[Samuel Thibault]

Hello,

Simon Marchi, on mar. 26 déc. 2017 21:48:23 -0500, wrote:
> We would also need a ChangeLog entry.

Well, I had put a ChangeLog entry at the top of the posted file. Is
something else needed?  In the patch I have just sent to the list I have
directly put the gdb/ChangeLog update in addition to the commit message
log.  If that is what is wanted, then the gdb/CONTRIBUTE file should be
update, otherwise frustration happens on both sides.

Samuel

Re: hurd: PIE support

[Simon Marchi]

On 2017-12-22 11:55 AM, Samuel Thibault wrote:
> Hello,
> 
> PIE is being pushed more and more, so we have to support it in the Hurd
> port :)
> 
> The simplest way to fix things is to provide gdb with the entry address
> through auxv. The attached patch implements this. Could you have a look
> soon?
> 
> Samuel

Hi Samuel,

The patch looks good to me, although I can't really try it and confirm it
works.

Some coding style comments: unless the variable is logically a boolean, we
try to be explicit in our comparisons:

  if (pointer != NULL)
  if (integer != 0)

instead of

  if (pointer)
  if (integer)

So if you could fix that throughout the patch, it would be appreciated.  We
would also need a ChangeLog entry.  If you can add that, as well as a proper
title and commit message, and re-submit using git-send-email, it would be
appreciated.  More comments below.


--- gdb-7.12.orig/gdb/gnu-nat.c
+++ gdb-7.12/gdb/gnu-nat.c
@@ -52,6 +52,8 @@ extern "C"
 #include 
 #include 
 #include 
+#include 
+#include 

 #include "inferior.h"
 #include "symtab.h"
@@ -2542,6 +2544,61 @@ gnu_xfer_memory (gdb_byte *readbuf, cons
 }
 }

+/* GNU does not have auxv, but we can at least fake the AT_ENTRY entry for PIE
+   binaries.  */
+static enum target_xfer_status
+gnu_xfer_auxv (gdb_byte *readbuf, const gdb_byte *writebuf,
+  CORE_ADDR memaddr, ULONGEST len, ULONGEST *xfered_len)
+{
+  task_t task = (gnu_current_inf
+? (gnu_current_inf->task
+   ? gnu_current_inf->task->port : 0)
+: 0);
+  process_t proc;
+  int res;
+  kern_return_t err;
+  vm_address_t entry;
+  ElfW(auxv_t) auxv[2];
+
+  if (task == MACH_PORT_NULL)
+return TARGET_XFER_E_IO;
+  if (writebuf != NULL)
+return TARGET_XFER_E_IO;
+
+  err = proc_task2proc (proc_server, task, );
+  if (err)
+return TARGET_XFER_E_IO;
+
+  /* Get entry from proc server.  */
+  err = proc_get_entry (proc, );
+  if (err)
+return TARGET_XFER_E_IO;
+
+  /* Fake auxv entry.  */
+  auxv[0].a_type = AT_ENTRY;
+  auxv[0].a_un.a_val = entry;
+  auxv[1].a_type = AT_NULL;
+  auxv[1].a_un.a_val = 0;
+
+  inf_debug (gnu_current_inf, "reading auxv %s[%s] --> %s",
+paddress (target_gdbarch (), memaddr), pulongest (len),
+host_address_to_string (readbuf));
+
+  if (memaddr == sizeof(auxv))
+return TARGET_XFER_EOF;
+
+  if (memaddr > sizeof(auxv))
+return TARGET_XFER_E_IO;

Can we do these two checks earlier, so we don't do unnecessary work in
those cases?

Also, please add a space after sizeof (as if it was a function call).

+  if (memaddr + len > sizeof(auxv))
+len = sizeof(auxv) - memaddr;
+
+  memcpy (readbuf, (gdb_byte*)  + memaddr, len);

Space before *.

+  *xfered_len = len;
+
+  return TARGET_XFER_OK;
+}
+
 /* Target to_xfer_partial implementation.  */

 static enum target_xfer_status
@@ -2554,6 +2611,8 @@ gnu_xfer_partial (struct target_ops *ops
 {
 case TARGET_OBJECT_MEMORY:
   return gnu_xfer_memory (readbuf, writebuf, offset, len, xfered_len);
+case TARGET_OBJECT_AUXV:
+  return gnu_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
 default:
   return TARGET_XFER_E_IO;
 }

Thanks,

Simon