Opened 9 years ago

Closed 9 years ago

#271 closed defect (fixed)

build failure with sunstudio in rrttl_python.cc and rrtype_python.cc

Reported by: jreed Owned by: jelte
Priority: medium Milestone: 06. 4th Incremental Release
Component: libdns++ Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Build failure on sunstudio

Making all in python
Making all in tests
source='libdns_python.cc' object='libdns_python_la-libdns_python.lo' libtool=yes \
DEPDIR=.deps depmode=none /bin/bash ../../../../depcomp \
/bin/bash ../../../../libtool  --tag=CXX   --mode=compile /usr/bin/CC -DHAVE_CONFIG_H -I. -I../../../..  -I../../../../src/lib -I../../../../src/lib -I/udir/jreed/pkg/include/python3.1 -I/udir/jreed/pkg/include/python3.1 -D_XPG4_2 -D__EXTENSIONS__ -I/udir/jreed/pkg/include -I../../../../ext/asio  -g -library=stlport4 -features=tmplife -features=tmplrefstatic -c -o libdns_python_la-libdns_python.lo `test -f 'libdns_python.cc' || echo './'`libdns_python.cc
libtool: compile:  /usr/bin/CC -DHAVE_CONFIG_H -I. -I../../../.. -I../../../../src/lib -I../../../../src/lib -I/udir/jreed/pkg/include/python3.1 -I/udir/jreed/pkg/include/python3.1 -D_XPG4_2 -D__EXTENSIONS__ -I/udir/jreed/pkg/include -I../../../../ext/asio -g -library=stlport4 -features=tmplife -features=tmplrefstatic -c libdns_python.cc  -KPIC -DPIC -o .libs/libdns_python_la-libdns_python.o
"name_python.cc", line 239: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"rrclass_python.cc", line 117: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"rrtype_python.cc", line 143: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"rrtype_python.cc", line 206: Error: An integer constant expression is required within the array subscript operator.
"rrttl_python.cc", line 104: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"rrttl_python.cc", line 165: Error: An integer constant expression is required within the array subscript operator.
"rdata_python.cc", line 109: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"rrset_python.cc", line 111: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"question_python.cc", line 93: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"message_python.cc", line 265: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"message_python.cc", line 549: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
"message_python.cc", line 1134: Warning (Anachronism): Using _object*(*)(_object*) to initialize extern "C" _object*(*)(_object*).
2 Error(s) and 10 Warning(s) detected.
*** Error code 1
make: Fatal error: Command failed for target `libdns_python_la-libdns_python.lo'
Current working directory /export/home/users/jreed/builder/work/BIND10-no-gtest/20100701032000-Solaris10-sparc-Sunstudio/build/src/lib/dns/python
*** Error code 1

Subtickets

Attachments (2)

271.diff (2.0 KB) - added by jreed 9 years ago.
patch from jelte
dnspython.diff (2.0 KB) - added by jinmei 9 years ago.

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by jreed

patch from jelte

comment:1 follow-up: Changed 9 years ago by jreed

See 271.diff patch from jelte.

And:

Index: src/lib/dns/python/Makefile.am
===================================================================
--- src/lib/dns/python/Makefile.am      (revision 2376)
+++ src/lib/dns/python/Makefile.am      (working copy)
@@ -1,6 +1,9 @@
 SUBDIRS = tests
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+if USE_GXX
+AM_CPPFLAGS += -Wno-write-strings
+endif

But I don't know if this is needed -- not used on Sunstudio.

comment:2 in reply to: ↑ 1 Changed 9 years ago by jinmei

  • Component changed from Unclassified to DNSPacket API
  • Milestone set to 06. 4th Incremental Release
  • Owner set to jelte
  • Status changed from new to reviewing

Replying to jreed:

See 271.diff patch from jelte.

This patch isn't exception safe. Please see my counter patch (attaching it).

As a general rule of thumb, new'ing a temporary region this way almost always introduces this type of bug and should be avoided.

+if USE_GXX
+AM_CPPFLAGS += -Wno-write-strings
+endif

But I don't know if this is needed -- not used on Sunstudio.

This may not be necessary to make it compile, but it's better to have it. Please commit that part.

I'll give this ticket to Jelte for the main patch.

Changed 9 years ago by jinmei

comment:3 follow-up: Changed 9 years ago by jelte

using a vector on the stack makes it less susceptible to memory leaks, but not exception-safe (at least, not without an exception-safe allocator thrown into its constructor), it would only result in a different type of bad_alloc being thrown.

comment:4 in reply to: ↑ 3 Changed 9 years ago by jinmei

Replying to jelte:

using a vector on the stack makes it less susceptible to memory leaks, but not exception-safe (at least, not without an exception-safe allocator thrown into its constructor), it would only result in a different type of bad_alloc being thrown.

I'm not sure what kind of failure mode you're talking about...do you mean the construction of vector could throw an exception? If so, that's true, and, realistically, the pythong program calling this wrapper would simply die in that case. I wouldn't bother to solve this failure mode, and this is not what I meant by "exception-safe" (I'd use "exception-free" in that sense). Maybe we are using different definitions of exception-safe?

On the other hand, the original 271.diff has a more critical problem, if I understand it correctly. See, for example, RRTTL_init(). This is called in the context of "from-wire" construction, right? So if the wire data is bogus and (e.g.) has an insufficent length of data, the RRTTL constructor will throw an exception, and the temporarily allocated memory will leak. This can be a remotely exploitable DoS. (although it wouldn't happen as long as we use the RRTTL constructor via a DNS message parser). The vector version doesn't have this problem.

comment:5 Changed 9 years ago by jelte

ahh, yes, i had exception-free in my mind, not exception-safe (blame it on the lack of morning coffee)

so yes, you're right. and I approve of your patch.

Shall I merge it?

comment:6 follow-up: Changed 9 years ago by jelte

Applied to trunk in r2409

comment:7 in reply to: ↑ 6 Changed 9 years ago by jinmei

Replying to jelte:

Applied to trunk in r2409

thanks, feel free to close it.

comment:8 Changed 9 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.