Opened 10 years ago

Closed 10 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 10 years ago.
patch from jelte
dnspython.diff (2.0 KB) - added by jinmei 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by jreed

patch from jelte

comment:1 follow-up: Changed 10 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 10 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 10 years ago by jinmei

comment:3 follow-up: Changed 10 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 10 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 10 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 10 years ago by jelte

Applied to trunk in r2409

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

Replying to jelte:

Applied to trunk in r2409

thanks, feel free to close it.

comment:8 Changed 10 years ago by jelte

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