[Bug libc/5070] New: glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[Bug libc/5070] New: glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[thunter at erggroup dot com]

When using getaddrinfo() in glibc-2.5 running on ARM9, I get the following:

../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion `sizeof 
(req) - __builtin_offsetof (struct req, pad) == 3' failed.

The problem was introduced in Revision 1.7 
of "libc/sysdeps/unix/sysv/linux/check_pf.c". This works fine on x86 because of 
different alignment and packing rules required by the x86 architecture.

I suggest to do something like this instead:
--------------
 static int
 make_request (int fd, pid_t pid, bool *seen_ipv4, bool *seen_ipv6,
 	      struct in6addrinfo **in6ai, size_t *in6ailen)
 {
   struct req
   {
     struct nlmsghdr nlh;
     struct rtgenmsg g;
-    /* struct rtgenmsg consists of a single byte.  This means there
-       are three bytes of padding included in the REQ definition.
-       We make them explicit here.  */
-    char pad[3];
   } req;
   struct sockaddr_nl nladdr;
 
+  memset (&req, '\0', sizeof (req));
   req.nlh.nlmsg_len = sizeof (req);
   req.nlh.nlmsg_type = RTM_GETADDR;
   req.nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
   req.nlh.nlmsg_pid = 0;
   req.nlh.nlmsg_seq = time (NULL);
   req.g.rtgen_family = AF_UNSPEC;
 
-  assert (sizeof (req) - offsetof (struct req, pad) == 3);
-  memset (req.pad, '\0', sizeof (req.pad));
 
   memset (&nladdr, '\0', sizeof (nladdr));
   nladdr.nl_family = AF_NETLINK;
 
   if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
 				    (struct sockaddr *) &nladdr,
 				    sizeof (nladdr))) < 0)
     return -1;
--------------

Regards

Tom Hunter

-- 
           Summary: glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68:
                    make_request: Assertion fails
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: thunter at erggroup dot com
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: arm-9tdmi-linux-gnu
  GCC host triplet: arm-9tdmi-linux-gnu
GCC target triplet: arm-9tdmi-linux-gnu


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2007-10-02 04:01 -------
It's working fine everywhere but this carp architectures.  I'm not going to make
the code perform worse just because of Arm.  Providing your own copy of that
file if you care.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[thunter at erggroup dot com]

------- Additional Comments From thunter at erggroup dot com  2007-10-02 05:45 -------
ARM is one of the architectures supported by glibc. You may not like it, but it 
is a fact.

Independently of the architecture, the padding done is not valid. You can't 
(and shouldn't) make any assumption about the alignment and associated padding 
done by the compiler for any architecture. GCC is free to change the alignment 
rules in any future versions. It seems rather silly to have the assert() which 
is meant to verify at runtime that your invalid assumption holds true.

I would also suggest that you don't use structures to format packets for 
networking. Packets for networking should be treated as byte streams to avoid 
any alignment/padding/byte-order issues. A standard way of doing this is 
something like this:

unsigned char buf[MaxPacketSize];
unsigned char *bp;
int skt;
size_t len;

...
bp = buf;
*bp++ = ...;
*bp++ = ...;
*bp++ = ...;
...
len = send(skt, buf, bp - buf, 0);
...


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2007-10-02 14:20 -------
No, Arm is not supported.  It might be supported in the ports but not in the
main package.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |WONTFIX


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[thunter at erggroup dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roland at gnu dot org


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[thunter at erggroup dot com]

------- Additional Comments From thunter at erggroup dot com  2007-10-03 03:12 -------
This is a bona fide bug where Ulrich Drepper makes invalid assumptions about 
code generated by GCC. Could some else please review my suggested fix and 
commit the patch. Unfortunately it appears that Ulrich does not understand the 
need for portable code and writes for Linux on x86 platforms only.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug ports/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2007-10-03 06:13 -------
This has nothing to do with "x86 only".  All ABIs designed by people who have a
bit of understanding require no change.  Any change will negatively impact well
designed architectures for the sole benefit of this embedded crap.  But your own
version of the file in the add-on.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libc                        |ports


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug ports/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[jsm28 at gcc dot gnu dot org]

------- Additional Comments From jsm28 at gcc dot gnu dot org  2007-10-03 12:10 -------
The file already exists in ports, since:

2007-01-08  Daniel Jacobowitz  <dan@codesourcery.com>

        * sysdeps/unix/sysv/linux/arm/check_pf.c: New file.
        * sysdeps/unix/sysv/linux/arm/eabi/check_pf.c: New file.

(The issue only applies for old-ABI, EABI does not have this ABI peculiarity so
the EABI version just includes the generic one.)


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug ports/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[ikalvachev at gmail dot com]

------- Additional Comments From ikalvachev at gmail dot com  2007-10-05 23:42 -------
If I understand correctly, the bug is that the arm aligns to 8 bytes, not 4.
This causes bigger align, that is not properly cleared but still would be send
over the network.

Instead of arguing how to clear the padding set by the compiler, I think it
would be much better to not send it at all. There is simple portable way to get
the size of the whole structure without padding and it is already used in the
assert().

Using offsetof(struct req,pad) instead of sizeof(req) would always give us the
correct size of the structure without the padding. It would still require at
least one byte padding (that we can ignore).


This solution is not only more portable, it is also faster.
And this is what I care about 
(I've never touched arm so far).


Here is sample patch against glibc 2.6.1, it applies without problems to current
cvs HEAD r1.12.

---------
--- check_pf.c.old      2007-04-25 19:05:18.000000000 +0300
+++ check_pf.c  2007-10-06 00:54:45.000000000 +0300
@@ -53,21 +53,18 @@ make_request (int fd, pid_t pid, bool *s
     struct rtgenmsg g;
     /* struct rtgenmsg consists of a single byte.  This means there
        are three bytes of padding included in the REQ definition.
-       We make them explicit here.  */
-    char pad[3];
+       We use pad as a mark for the size of the data we need.  */
+    char pad;
   } req;
   struct sockaddr_nl nladdr;
 
-  req.nlh.nlmsg_len = sizeof (req);
+  req.nlh.nlmsg_len = offsetof (struct req, pad);
   req.nlh.nlmsg_type = RTM_GETADDR;
   req.nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
   req.nlh.nlmsg_pid = 0;
   req.nlh.nlmsg_seq = time (NULL);
   req.g.rtgen_family = AF_UNSPEC;
 
-  assert (sizeof (req) - offsetof (struct req, pad) == 3);
-  memset (req.pad, '\0', sizeof (req.pad));
-
   memset (&nladdr, '\0', sizeof (nladdr));
   nladdr.nl_family = AF_NETLINK;
 
@@ -94,7 +91,7 @@ make_request (int fd, pid_t pid, bool *s
 
   struct iovec iov = { buf, buf_size };
 
-  if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0,
+  if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, offsetof (struct req,
pad), 0,
                                    (struct sockaddr *) &nladdr,
                                    sizeof (nladdr))) < 0)
     goto out_fail;
---------

Thank you for your patience.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug ports/5070] glibc-2.5: ../sysdeps/unix/sysv/linux/check_pf.c:68: make_request: Assertion fails

[jsm28 at gcc dot gnu dot org]

------- Additional Comments From jsm28 at gcc dot gnu dot org  2009-03-18 12:20 -------
I believe this was fixed by:

2008-04-21  Daniel Jacobowitz  <dan@codesourcery.com>

        * sysdeps/unix/sysv/linux/arm/check_pf.c: Update from generic
        version.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=5070

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.