| Message ID | 5213B6DC.4040608@redhat.com |
|---|---|
| State | New |
| Headers | show |
That fixed it, thanks! Attached is the latest patch (Florian, I will
send you the regenerated Makefile.in and configure separately).
Please review and let me know if this is OK to commit!
-- Caroline
cmtice@google.com
2013-08-20 Caroline Tice <cmtice@google.com>
* Makefile.am (DEFS): Add "@DEFS@", to inherit defintions.
* Makefile.in: Regenerate.
* configure.ac: Add check for __secure_getenv and secure_getenv.
* configure: Regenerate.
* vtv_utils.cc : Include stdlib.h
(HAVE_SECURE_GETENV): Add checks and definitions for secure_getenv.
(log_dirs): Remove file static constant.
(__vtv_open_log): Increase size of log file name. Add the user
and process ids to the file name. Do not put the log files in /tmp.
Instead try to get the directory name from an environment variable; if
that fails try to use stderr. Add O_NOFOLLOW to the flags
for 'open'. Update function comment.
* vtv_rts.cc (log_memory_protection_data): Remove %d from file name.
On Tue, Aug 20, 2013 at 11:35 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/20/2013 12:24 AM, Caroline Tice wrote:
>>
>> Hi All,
>>
>> I could really use some help here from someone who has a better
>> understanding of how the config/Makefile system works than I do.
>>
>> In my libvtv/configure.ac file, I have:
>>
>> AC_GNU_SOURCE
>> AC_CHECK_FUNCS([__secure_getenv])
>>
>> AC_GNU_SOURCE
>> AC_CHECK_FUNCS([secure_getenv])
>
>
> There's nothing in Makefile.am that makes use of the substitution variables,
> so configure doesn't actually emit code to define the matching preprocessor
> macros.
>
> I tried to change that, by putting "DEFS = @DEFS@" into libvtv/Makefile.am
> (libvtv/Makefile.in is derived from that). The diff below is slightly
> garbled, but you get the idea.
>
> diff --git a/libvtv/Makefile.am b/libvtv/Makefile.am
> index 73acfb4..567bd81 100644
> --- a/libvtv/Makefile.am
> +++ b/libvtv/Makefile.am
> @@ -30,7 +30,7 @@ ACLOCAL_AMFLAGS = -I .. -I ../config
> # May be used by toolexeclibdir.
> gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
>
> -DEFS =
> +DEFS = @DEFS@
> AM_CPPFLAGS = -I$(top_srcdir)/../include
> AM_CFLAGS = $(XCFLAGS)
> AM_CCASFLAGS = $(XCFLAGS)
>
>
> But I ended up with:
>
> libtool: compile: /home/fw/src/gnu/gcc/build/./gcc/xgcc
> -B/home/fw/src/gnu/gcc/build/./gcc/ "-DPACKAGE_NAME=\"GNU Vtable
> Verification Runtime Library\"" -DPACKAGE_TARNAME=\"libvtv\"
> -DPACKAGE_VERSION=\"1.0\" "-DPACKAGE_STRING=\"GNU Vtable Verification
> Runtime Library 1.0\"" -DPACKAGE_BUGREPORT=\"\"
> -DPACKAGE_URL=\"http://www.gnu.org/software/libvtv/\" -DPACKAGE=\"libvtv\"
> -DVERSION=\"1.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1
> -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1
> -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1
> -DLT_OBJDIR=\".libs/\" -D__EXTENSIONS__=1 -D_ALL_SOURCE=1 -D_GNU_SOURCE=1
> -D_POSIX_PTHREAD_SEMANTICS=1 -D_TANDEM_SOURCE=1 -DHAVE___SECURE_GETENV=1 -I.
> -I../../../git/libvtv -I../../../git/libvtv/../include -D_GNU_SOURCE -Wall
> -Wextra -fno-exceptions -Wl,-u_vtable_map_vars_start,-u_vtable_map_vars_end
> -g -O2 -D_GNU_SOURCE -MT vtv_rts.lo -MD -MP -MF .deps/vtv_rts.Tpo -c
> ../../../git/libvtv/vtv_rts.cc -fPIC -DPIC -o .libs/vtv_rts.o
> ../../../git/libvtv/vtv_rts.cc:134:28: fatal error: bits/c++config.h: No
> such file or directory
> #include <bits/c++config.h>
> ^
> (The caret is at the wrong position, IMHO.)
>
> But I suspect something went wrong with regenerating Makefile.in. Have you
> got automake 1.11? Then you could make the change, regenerate Makefile.in,
> and try again.
>
>
> --
> Florian Weimer / Red Hat Product Security Team
On 08/20/2013 09:15 PM, Caroline Tice wrote: > That fixed it, thanks! Attached is the latest patch (Florian, I will > send you the regenerated Makefile.in and configure separately). > Please review and let me know if this is OK to commit! As the libvtv reviewer, you don't need permission to commit your changes. :-) But I bootstrapped on x86_64-debian-linux-gnu, and can confirm that this now works as intended; __secure_getenv is picked up. One minor nit: Index: libvtv/vtv_utils.cc =================================================================== --- libvtv/vtv_utils.cc (revision 201802) +++ libvtv/vtv_utils.cc (working copy) + decriptor. There's a stray space at the end of this line.
Ok, committed (with the space fix). -- Caroline Tice cmtice@google.com On Tue, Aug 20, 2013 at 12:43 PM, Florian Weimer <fweimer@redhat.com> wrote: > On 08/20/2013 09:15 PM, Caroline Tice wrote: >> >> That fixed it, thanks! Attached is the latest patch (Florian, I will >> send you the regenerated Makefile.in and configure separately). >> Please review and let me know if this is OK to commit! > > > As the libvtv reviewer, you don't need permission to commit your changes. > :-) > > But I bootstrapped on x86_64-debian-linux-gnu, and can confirm that this now > works as intended; __secure_getenv is picked up. > > One minor nit: > > Index: libvtv/vtv_utils.cc > =================================================================== > --- libvtv/vtv_utils.cc (revision 201802) > +++ libvtv/vtv_utils.cc (working copy) > > + decriptor. > > There's a stray space at the end of this line. > > > -- > Florian Weimer / Red Hat Product Security Team
On Tue, 20 Aug 2013, Florian Weimer wrote: > As the libvtv reviewer, you don't need permission to commit your > changes. :-) Actually, reviewers do need someone else's approval for their own changes (unlike maintainers and of course not for trivial changes). Not a biggie in this case, just wanted to clarify. Gerald
On 08/25/2013 09:33 PM, Gerald Pfeifer wrote: > On Tue, 20 Aug 2013, Florian Weimer wrote: >> As the libvtv reviewer, you don't need permission to commit your >> changes. :-) > > Actually, reviewers do need someone else's approval for their own > changes (unlike maintainers and of course not for trivial changes). Ah, but I'm not a global reviewer, so I couldn't approve the change anyway. :-) Caroline, I think your patch in <http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00619.html> doesn't match the ChangeLog update and what was approved by Jeff. It should have gone into the "Various Maintainers" section.
diff --git a/libvtv/Makefile.am b/libvtv/Makefile.am index 73acfb4..567bd81 100644 --- a/libvtv/Makefile.am +++ b/libvtv/Makefile.am @@ -30,7 +30,7 @@ ACLOCAL_AMFLAGS = -I .. -I ../config # May be used by toolexeclibdir. gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER) -DEFS = +DEFS = @DEFS@ AM_CPPFLAGS = -I$(top_srcdir)/../include AM_CFLAGS = $(XCFLAGS) AM_CCASFLAGS = $(XCFLAGS)