[PATCH] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[PATCH] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

Hi,

As discussed previously, here a patch that tries to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary. Additionally, I've also reverted my previous change to cc-with-tweaks.sh that added the -data-directory flag, since it is no longer necessary.

Thank you,

Yit
September 18, 2012



[-- Attachment #2: gdb-relative-data-directory.txt --]
[-- Type: text/plain, Size: 5685 bytes --]

gdb/ChangeLog

2012-09-18  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Try to initialize data-directory by first searching for
	"data-directory" in the same directory as the gdb binary.
	* contrib/cc-with-tweaks.sh (GDB): Revert -data-directory
	additions that is now unnecessary.
	* main.c (relocate_path): Add an isdir argument, and check that
	the relocated file/directory exists.
	(relocate_gdb_directory): Remove the directory check that is
	subsumed by relocate_path.
	(get_init_files): Remove the file check that is subsumed by
	relocate_path.
	(relocate_gdb_data_directory): New function similar to
	relocate_gdb_directory, but specifically for data-directory.
	(captured_main): Call relocate_gdb_data_directory to initialize
	gdb_datadir.

diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh
--- a/gdb/contrib/cc-with-tweaks.sh
+++ b/gdb/contrib/cc-with-tweaks.sh
@@ -19,8 +19,7 @@
 # This program requires gdb and objcopy in addition to gcc.
 # The default values are gdb from the build tree and objcopy from $PATH.
 # They may be overridden by setting environment variables GDB and OBJCOPY
-# respectively.  Note that GDB should contain the gdb binary as well as the
-# -data-directory flag, e.g., "foo/gdb -data-directory foo/data-directory".
+# respectively.
 # We assume the current directory is either $obj/gdb or $obj/gdb/testsuite.
 #
 # Example usage:
@@ -47,13 +46,13 @@
 then
     if [ -f ./gdb ]
     then
-	GDB="./gdb -data-directory data-directory"
+	GDB="./gdb"
     elif [ -f ../gdb ]
     then
-	GDB="../gdb -data-directory ../data-directory"
+	GDB="../gdb"
     elif [ -f ../../gdb ]
     then
-	GDB="../../gdb -data-directory ../../data-directory"
+	GDB="../../gdb"
     else
 	echo "$myname: unable to find usable gdb" >&2
 	exit 1
diff --git a/gdb/main.c b/gdb/main.c
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -91,17 +91,34 @@
 
 static void print_gdb_help (struct ui_file *);
 
-/* Relocate a file or directory.  PROGNAME is the name by which gdb
-   was invoked (i.e., argv[0]).  INITIAL is the default value for the
-   file or directory.  FLAG is true if the value is relocatable, false
-   otherwise.  Returns a newly allocated string; this may return NULL
-   under the same conditions as make_relative_prefix.  */
+/* Relocate a file or directory, checking if it exists.  PROGNAME is the
+   name by which gdb was invoked (i.e., argv[0]).  INITIAL is the default
+   value for the file or directory.  ISDIR is true if INITIAL is a
+   directory.  FLAG is true if the value is relocatable, false otherwise.
+   Returns a newly allocated string; this may return NULL under the same
+   conditions as make_relative_prefix, or if the relocated path does not
+   exist.  */
 
 static char *
-relocate_path (const char *progname, const char *initial, int flag)
+relocate_path (const char *progname, const char *initial, int isdir,
+	       int flag)
 {
   if (flag)
-    return make_relative_prefix (progname, BINDIR, initial);
+    {
+      char *path;
+      path = make_relative_prefix (progname, BINDIR, initial);
+      if (path)
+	{
+	  struct stat s;
+
+	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode)))
+	    {
+	      xfree (path);
+	      path = NULL;
+	    }
+	}
+      return path;
+    }
   return xstrdup (initial);
 }
 
@@ -116,19 +133,52 @@
 {
   char *dir;
 
-  dir = relocate_path (gdb_program_name, initial, flag);
-  if (dir)
+  dir = relocate_path (gdb_program_name, initial, 1, flag);
+  if (!dir)
+    dir = xstrdup (initial);
+
+  /* Canonicalize the directory.  */
+  if (*dir)
     {
-      struct stat s;
+      char *canon_sysroot = lrealpath (dir);
 
-      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
+      if (canon_sysroot)
 	{
 	  xfree (dir);
-	  dir = NULL;
+	  dir = canon_sysroot;
 	}
     }
+
+  return dir;
+}
+
+/* Like relocate_gdb_path, but specifically for data-directory. */
+
+static char *
+relocate_gdb_data_directory (void)
+{
+  char *dir;
+
+  /* First try to find "data-directory" in the same directory as gdb.
+
+    Use relocate_path only to resolve the parent directory of
+    gdb_program_name (i.e., based on PATH if necessary); relocate_path
+    (gdb_program_name, BINDIR "/data-directory") cannot be used to resolve
+    data-directory as it returns a path relative to the _grandparent
+    directory_ of gdb_program_name (munging the parent directory).  */
+
+  dir = relocate_path (gdb_program_name, BINDIR, 1, 1);
+  if (dir)
+    dir = reconcat (dir, dir, SLASH_STRING, "data-directory", NULL);
+
+  /* Then try to find GDB_DATADIR relocated relative to gdb. */
   if (!dir)
-    dir = xstrdup (initial);
+    dir = relocate_path (gdb_program_name, GDB_DATADIR, 1,
+			 GDB_DATADIR_RELOCATABLE);
+
+  /* Otherwise use GDB_DATADIR as is. */
+  if (!dir)
+    dir = xstrdup (GDB_DATADIR);
 
   /* Canonicalize the directory.  */
   if (*dir)
@@ -162,15 +212,15 @@
 
   if (!initialized)
     {
-      struct stat homebuf, cwdbuf, s;
+      struct stat homebuf, cwdbuf;
       char *homedir, *relocated_sysgdbinit;
 
       if (SYSTEM_GDBINIT[0])
 	{
 	  relocated_sysgdbinit = relocate_path (gdb_program_name,
-						SYSTEM_GDBINIT,
+						SYSTEM_GDBINIT, 0,
 						SYSTEM_GDBINIT_RELOCATABLE);
-	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
+	  if (relocated_sysgdbinit)
 	    sysgdbinit = relocated_sysgdbinit;
 	  else
 	    xfree (relocated_sysgdbinit);
@@ -363,8 +413,7 @@
   debug_file_directory = relocate_gdb_directory (DEBUGDIR,
 						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
-					GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_data_directory ();
 
 #ifdef WITH_PYTHON_PATH
   {

Re: [PATCH] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Jan Kratochvil]

On Tue, 18 Sep 2012 22:32:40 +0200, Khoo Yit Phang wrote:
> As discussed previously, here a patch that tries to initialize
> data-directory by first searching for "data-directory" in the same directory
> as the gdb binary.

I do not see how is it related to the 'isdir' addition, IMO 'isdir' is an
unrelated patch which should be posted separately.


> Additionally, I've also reverted my previous change to cc-with-tweaks.sh
> that added the -data-directory flag, since it is no longer necessary.

It can be kept there, it does not harm anything and it then does not depend on
a bit magic relocate_gdb_directory.



Thanks,
Jan


> gdb/ChangeLog
> 
> 2012-09-18  Khoo Yit Phang  <khooyp@cs.umd.edu>
> 
> 	Try to initialize data-directory by first searching for
> 	"data-directory" in the same directory as the gdb binary.
> 	* contrib/cc-with-tweaks.sh (GDB): Revert -data-directory
> 	additions that is now unnecessary.
> 	* main.c (relocate_path): Add an isdir argument, and check that
> 	the relocated file/directory exists.
> 	(relocate_gdb_directory): Remove the directory check that is
> 	subsumed by relocate_path.
> 	(get_init_files): Remove the file check that is subsumed by
> 	relocate_path.
> 	(relocate_gdb_data_directory): New function similar to
> 	relocate_gdb_directory, but specifically for data-directory.
> 	(captured_main): Call relocate_gdb_data_directory to initialize
> 	gdb_datadir.
> 
> diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh
> --- a/gdb/contrib/cc-with-tweaks.sh
> +++ b/gdb/contrib/cc-with-tweaks.sh
> @@ -19,8 +19,7 @@
>  # This program requires gdb and objcopy in addition to gcc.
>  # The default values are gdb from the build tree and objcopy from $PATH.
>  # They may be overridden by setting environment variables GDB and OBJCOPY
> -# respectively.  Note that GDB should contain the gdb binary as well as the
> -# -data-directory flag, e.g., "foo/gdb -data-directory foo/data-directory".
> +# respectively.
>  # We assume the current directory is either $obj/gdb or $obj/gdb/testsuite.
>  #
>  # Example usage:
> @@ -47,13 +46,13 @@
>  then
>      if [ -f ./gdb ]
>      then
> -	GDB="./gdb -data-directory data-directory"
> +	GDB="./gdb"
>      elif [ -f ../gdb ]
>      then
> -	GDB="../gdb -data-directory ../data-directory"
> +	GDB="../gdb"
>      elif [ -f ../../gdb ]
>      then
> -	GDB="../../gdb -data-directory ../../data-directory"
> +	GDB="../../gdb"
>      else
>  	echo "$myname: unable to find usable gdb" >&2
>  	exit 1
> diff --git a/gdb/main.c b/gdb/main.c
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -91,17 +91,34 @@
>  
>  static void print_gdb_help (struct ui_file *);
>  
> -/* Relocate a file or directory.  PROGNAME is the name by which gdb
> -   was invoked (i.e., argv[0]).  INITIAL is the default value for the
> -   file or directory.  FLAG is true if the value is relocatable, false
> -   otherwise.  Returns a newly allocated string; this may return NULL
> -   under the same conditions as make_relative_prefix.  */
> +/* Relocate a file or directory, checking if it exists.  PROGNAME is the
> +   name by which gdb was invoked (i.e., argv[0]).  INITIAL is the default
> +   value for the file or directory.  ISDIR is true if INITIAL is a
> +   directory.  FLAG is true if the value is relocatable, false otherwise.
> +   Returns a newly allocated string; this may return NULL under the same
> +   conditions as make_relative_prefix, or if the relocated path does not
> +   exist.  */

GDB comments are formatted to textwidth=72 which may explain this excessive
reformatting.  Try to keep textwidth=72 instead of changes back and forth.


>  
>  static char *
> -relocate_path (const char *progname, const char *initial, int flag)
> +relocate_path (const char *progname, const char *initial, int isdir,
> +	       int flag)
>  {
>    if (flag)
> -    return make_relative_prefix (progname, BINDIR, initial);
> +    {
> +      char *path;
> +      path = make_relative_prefix (progname, BINDIR, initial);
> +      if (path)
> +	{
> +	  struct stat s;
> +
> +	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode)))
> +	    {
> +	      xfree (path);
> +	      path = NULL;
> +	    }
> +	}
> +      return path;
> +    }
>    return xstrdup (initial);
>  }
>  
> @@ -116,19 +133,52 @@
>  {
>    char *dir;
>  
> -  dir = relocate_path (gdb_program_name, initial, flag);
> -  if (dir)
> +  dir = relocate_path (gdb_program_name, initial, 1, flag);
> +  if (!dir)
> +    dir = xstrdup (initial);
> +
> +  /* Canonicalize the directory.  */
> +  if (*dir)
>      {
> -      struct stat s;
> +      char *canon_sysroot = lrealpath (dir);
>  
> -      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
> +      if (canon_sysroot)
>  	{
>  	  xfree (dir);
> -	  dir = NULL;
> +	  dir = canon_sysroot;
>  	}
>      }
> +
> +  return dir;
> +}
> +
> +/* Like relocate_gdb_path, but specifically for data-directory. */
> +
> +static char *
> +relocate_gdb_data_directory (void)
> +{
> +  char *dir;
> +
> +  /* First try to find "data-directory" in the same directory as gdb.
> +
> +    Use relocate_path only to resolve the parent directory of
> +    gdb_program_name (i.e., based on PATH if necessary); relocate_path
> +    (gdb_program_name, BINDIR "/data-directory") cannot be used to resolve
> +    data-directory as it returns a path relative to the _grandparent
> +    directory_ of gdb_program_name (munging the parent directory).  */
> +
> +  dir = relocate_path (gdb_program_name, BINDIR, 1, 1);
> +  if (dir)
> +    dir = reconcat (dir, dir, SLASH_STRING, "data-directory", NULL);
> +
> +  /* Then try to find GDB_DATADIR relocated relative to gdb. */
>    if (!dir)

It can be more clear by replacing this 'if (!dir)' by 'else'.


> -    dir = xstrdup (initial);
> +    dir = relocate_path (gdb_program_name, GDB_DATADIR, 1,
> +			 GDB_DATADIR_RELOCATABLE);

Why isn't here the original statement?
	dir = relocate_gdb_directory (GDB_DATADIR, GDB_DATADIR_RELOCATABLE);



> +
> +  /* Otherwise use GDB_DATADIR as is. */
> +  if (!dir)
> +    dir = xstrdup (GDB_DATADIR);

This can be moved to the 'else' block suggested above to simplify it.


>  
>    /* Canonicalize the directory.  */
>    if (*dir)
> @@ -162,15 +212,15 @@
>  
>    if (!initialized)
>      {
> -      struct stat homebuf, cwdbuf, s;
> +      struct stat homebuf, cwdbuf;
>        char *homedir, *relocated_sysgdbinit;
>  
>        if (SYSTEM_GDBINIT[0])
>  	{
>  	  relocated_sysgdbinit = relocate_path (gdb_program_name,
> -						SYSTEM_GDBINIT,
> +						SYSTEM_GDBINIT, 0,
>  						SYSTEM_GDBINIT_RELOCATABLE);
> -	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
> +	  if (relocated_sysgdbinit)
>  	    sysgdbinit = relocated_sysgdbinit;
>  	  else
>  	    xfree (relocated_sysgdbinit);
> @@ -363,8 +413,7 @@
>    debug_file_directory = relocate_gdb_directory (DEBUGDIR,
>  						 DEBUGDIR_RELOCATABLE);
>  
> -  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
> -					GDB_DATADIR_RELOCATABLE);
> +  gdb_datadir = relocate_gdb_data_directory ();
>  
>  #ifdef WITH_PYTHON_PATH
>    {

[PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

Hi Jan,

I've split my data-directory patch into two. Here's the first one in preparation for the next patch: it refactors relocate_path to also check if the relocated file or directory exists, to reduce some code redundancy.

Yit
September 19, 2012


[-- Attachment #2: refactor-relocate-path.txt --]
[-- Type: text/plain, Size: 2818 bytes --]

gdb/ChangeLog

2012-09-19  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Refactor relocate_path to also check if the relocated
	file/directory exists.
	* main.c (relocate_path): Add an isdir argument, and check that
	the relocated file/directory exists.
	(relocate_gdb_directory): Remove the directory check that is
	subsumed by relocate_path.
	(get_init_files): Remove the file check that is subsumed by
	relocate_path.

diff --git a/gdb/main.c b/gdb/main.c
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -91,17 +91,34 @@
 
 static void print_gdb_help (struct ui_file *);
 
-/* Relocate a file or directory.  PROGNAME is the name by which gdb
-   was invoked (i.e., argv[0]).  INITIAL is the default value for the
-   file or directory.  FLAG is true if the value is relocatable, false
-   otherwise.  Returns a newly allocated string; this may return NULL
-   under the same conditions as make_relative_prefix.  */
+/* Relocate a file or directory, checking if it exists.  PROGNAME is the
+   name by which gdb was invoked (i.e., argv[0]).  INITIAL is the default
+   value for the file or directory.  ISDIR is true if INITIAL is a
+   directory.  FLAG is true if the value is relocatable, false otherwise.
+   Returns a newly allocated string; this may return NULL under the same
+   conditions as make_relative_prefix, or if the relocated path does not
+   exist.  */
 
 static char *
-relocate_path (const char *progname, const char *initial, int flag)
+relocate_path (const char *progname, const char *initial, int isdir,
+	       int flag)
 {
   if (flag)
-    return make_relative_prefix (progname, BINDIR, initial);
+    {
+      char *path;
+      path = make_relative_prefix (progname, BINDIR, initial);
+      if (path)
+	{
+	  struct stat s;
+
+	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode)))
+	    {
+	      xfree (path);
+	      path = NULL;
+	    }
+	}
+      return path;
+    }
   return xstrdup (initial);
 }
 
@@ -116,17 +133,7 @@
 {
   char *dir;
 
-  dir = relocate_path (gdb_program_name, initial, flag);
-  if (dir)
-    {
-      struct stat s;
-
-      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
-	{
-	  xfree (dir);
-	  dir = NULL;
-	}
-    }
+  dir = relocate_path (gdb_program_name, initial, 1, flag);
   if (!dir)
     dir = xstrdup (initial);
 
@@ -162,15 +169,15 @@
 
   if (!initialized)
     {
-      struct stat homebuf, cwdbuf, s;
+      struct stat homebuf, cwdbuf;
       char *homedir, *relocated_sysgdbinit;
 
       if (SYSTEM_GDBINIT[0])
 	{
 	  relocated_sysgdbinit = relocate_path (gdb_program_name,
-						SYSTEM_GDBINIT,
+						SYSTEM_GDBINIT, 0,
 						SYSTEM_GDBINIT_RELOCATABLE);
-	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
+	  if (relocated_sysgdbinit)
 	    sysgdbinit = relocated_sysgdbinit;
 	  else
 	    xfree (relocated_sysgdbinit);

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Jan Kratochvil]

On Wed, 19 Sep 2012 21:53:16 +0200, Khoo Yit Phang wrote:
> -relocate_path (const char *progname, const char *initial, int flag)
> +relocate_path (const char *progname, const char *initial, int isdir,
> +	       int flag)
>  {
>    if (flag)
> -    return make_relative_prefix (progname, BINDIR, initial);
> +    {
> +      char *path;
> +      path = make_relative_prefix (progname, BINDIR, initial);
> +      if (path)
> +	{
> +	  struct stat s;
> +
> +	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode)))

OK just you could also check if !isdir, therefore:

	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode))
	      || (!isdir && S_ISDIR (s.st_mode)))

(I would write it differently but reviews here point to this style.)



Thanks,
Jan

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Eli Zaretskii]

> Date: Fri, 21 Sep 2012 20:26:37 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: GDB Patches <gdb-patches@sourceware.org>
> 
> 	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode))
> 	      || (!isdir && S_ISDIR (s.st_mode)))

a.k.a.

	  if (stat (path, &s) != 0 || isdir != S_ISDIR (s.st_mode))

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Jan Kratochvil]

On Fri, 21 Sep 2012 20:35:39 +0200, Eli Zaretskii wrote:
> > Date: Fri, 21 Sep 2012 20:26:37 +0200
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: GDB Patches <gdb-patches@sourceware.org>
> > 
> > 	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode))
> > 	      || (!isdir && S_ISDIR (s.st_mode)))
> 
> a.k.a.
> 
> 	  if (stat (path, &s) != 0 || isdir != S_ISDIR (s.st_mode))

It is not POSIX compliant:
	The macro evaluates to a non-zero value if the test is true

It would have to be written as:
  	  if (stat (path, &s) != 0 || !isdir != !S_ISDIR (s.st_mode))

I would find it still OK but this is why I wrote in the mail above:

# (I would write it differently but reviews here point to this style.)

Because I already posted some such patch here before and it was reviewed by
Joel to the form I wrote above.  Unfortunately I cannot find the mail now, it
is difficult to find a search pattern for this case.


Regards,
Jan

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Eli Zaretskii]

> Date: Fri, 21 Sep 2012 20:46:23 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: khooyp@cs.umd.edu, gdb-patches@sourceware.org
> 
> On Fri, 21 Sep 2012 20:35:39 +0200, Eli Zaretskii wrote:
> > > Date: Fri, 21 Sep 2012 20:26:37 +0200
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > Cc: GDB Patches <gdb-patches@sourceware.org>
> > > 
> > > 	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode))
> > > 	      || (!isdir && S_ISDIR (s.st_mode)))
> > 
> > a.k.a.
> > 
> > 	  if (stat (path, &s) != 0 || isdir != S_ISDIR (s.st_mode))
> 
> It is not POSIX compliant:
> 	The macro evaluates to a non-zero value if the test is true

Then

	if (stat (path, &s) != 0 
	    || (isdir != 0) != (S_ISDIR (s.st_mode) != 0))

will do.

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Andreas Schwab]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> It would have to be written as:
>   	  if (stat (path, &s) != 0 || !isdir != !S_ISDIR (s.st_mode))

A common idiom is to use !! when the other side is already boolean.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

Hi,

On Sep 21, 2012, at 3:09 PM, Andreas Schwab wrote:

> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
>> It would have to be written as:
>>  	  if (stat (path, &s) != 0 || !isdir != !S_ISDIR (s.st_mode))
> 
> A common idiom is to use !! when the other side is already boolean.


I like this idiom, it does seem to be used in GDB, and isdir should be a boolean. Here's a patch that uses it.

Yit
September 22, 2012


[-- Attachment #2: refactor-relocate-path.txt --]
[-- Type: text/plain, Size: 2819 bytes --]

gdb/ChangeLog

2012-09-21  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Refactor relocate_path to also check if the relocated
	file/directory exists.
	* main.c (relocate_path): Add an isdir argument, and check that
	the relocated file/directory exists.
	(relocate_gdb_directory): Remove the directory check that is
	subsumed by relocate_path.
	(get_init_files): Remove the file check that is subsumed by
	relocate_path.

diff --git a/gdb/main.c b/gdb/main.c
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -91,17 +91,34 @@
 
 static void print_gdb_help (struct ui_file *);
 
-/* Relocate a file or directory.  PROGNAME is the name by which gdb
-   was invoked (i.e., argv[0]).  INITIAL is the default value for the
-   file or directory.  FLAG is true if the value is relocatable, false
-   otherwise.  Returns a newly allocated string; this may return NULL
-   under the same conditions as make_relative_prefix.  */
+/* Relocate a file or directory, checking if it exists.  PROGNAME is the
+   name by which gdb was invoked (i.e., argv[0]).  INITIAL is the default
+   value for the file or directory.  ISDIR is true if INITIAL is a
+   directory.  FLAG is true if the value is relocatable, false otherwise.
+   Returns a newly allocated string; this may return NULL under the same
+   conditions as make_relative_prefix, or if the relocated path does not
+   exist.  */
 
 static char *
-relocate_path (const char *progname, const char *initial, int flag)
+relocate_path (const char *progname, const char *initial, int isdir,
+	       int flag)
 {
   if (flag)
-    return make_relative_prefix (progname, BINDIR, initial);
+    {
+      char *path;
+      path = make_relative_prefix (progname, BINDIR, initial);
+      if (path)
+	{
+	  struct stat s;
+
+	  if (stat (path, &s) != 0 || (isdir != !!S_ISDIR (s.st_mode)))
+	    {
+	      xfree (path);
+	      path = NULL;
+	    }
+	}
+      return path;
+    }
   return xstrdup (initial);
 }
 
@@ -116,17 +133,7 @@
 {
   char *dir;
 
-  dir = relocate_path (gdb_program_name, initial, flag);
-  if (dir)
-    {
-      struct stat s;
-
-      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
-	{
-	  xfree (dir);
-	  dir = NULL;
-	}
-    }
+  dir = relocate_path (gdb_program_name, initial, 1, flag);
   if (!dir)
     dir = xstrdup (initial);
 
@@ -162,15 +169,15 @@
 
   if (!initialized)
     {
-      struct stat homebuf, cwdbuf, s;
+      struct stat homebuf, cwdbuf;
       char *homedir, *relocated_sysgdbinit;
 
       if (SYSTEM_GDBINIT[0])
 	{
 	  relocated_sysgdbinit = relocate_path (gdb_program_name,
-						SYSTEM_GDBINIT,
+						SYSTEM_GDBINIT, 0,
 						SYSTEM_GDBINIT_RELOCATABLE);
-	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
+	  if (relocated_sysgdbinit)
 	    sysgdbinit = relocated_sysgdbinit;
 	  else
 	    xfree (relocated_sysgdbinit);

Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Jan Kratochvil]

On Sat, 22 Sep 2012 18:06:43 +0200, Khoo Yit Phang wrote:
> > A common idiom is to use !! when the other side is already boolean.
> 
> I like this idiom, it does seem to be used in GDB, and isdir should be
> a boolean. Here's a patch that uses it.
[...]
> +         if (stat (path, &s) != 0 || (isdir != !!S_ISDIR (s.st_mode)))

Sorry but it is not about what you like (*) but what has been concluded as
acceptable for the GDB codebase to keep it (possibly) easily contributable.

(*) I also use such constructs in my own code.  This is irrelevant here.


Thanks,
Jan

[PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

Hi Jan,

Here's the second of the split patch that actually implements the extra search for data-directory in the same directory as the gdb binary. I've dropped the reverting changes to cc-with-tweaks.sh as well.

Yit
September 19, 2012


[-- Attachment #2: gdb-relative-data-directory.txt --]
[-- Type: text/plain, Size: 2166 bytes --]

gdb/ChangeLog

2012-09-19  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Try to initialize data-directory by first searching for
	"data-directory" in the same directory as the gdb binary.
	* main.c (relocate_gdb_data_directory): New function similar to
	relocate_gdb_directory, but specifically for data-directory.
	(captured_main): Call relocate_gdb_data_directory to initialize
	gdb_datadir.

diff --git a/gdb/main.c b/gdb/main.c
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -152,6 +152,49 @@
   return dir;
 }
 
+/* Like relocate_gdb_path, but specifically for data-directory. */
+
+static char *
+relocate_gdb_data_directory (void)
+{
+  char *dir;
+
+  /* First try to find "data-directory" in the same directory as gdb.
+
+    Use relocate_path only to resolve the parent directory of
+    gdb_program_name (i.e., based on PATH if necessary); relocate_path
+    (gdb_program_name, BINDIR "/data-directory") cannot be used to resolve
+    data-directory as it returns a path relative to the _grandparent
+    directory_ of gdb_program_name (munging the parent directory).  */
+
+  dir = relocate_path (gdb_program_name, BINDIR, 1, 1);
+  if (dir)
+    dir = reconcat (dir, dir, SLASH_STRING, "data-directory", NULL);
+
+  /* Then try to find GDB_DATADIR relocated relative to gdb. */
+  if (!dir)
+    dir = relocate_path (gdb_program_name, GDB_DATADIR, 1,
+			 GDB_DATADIR_RELOCATABLE);
+
+  /* Otherwise use GDB_DATADIR as is. */
+  if (!dir)
+    dir = xstrdup (GDB_DATADIR);
+
+  /* Canonicalize the directory.  */
+  if (*dir)
+    {
+      char *canon_sysroot = lrealpath (dir);
+
+      if (canon_sysroot)
+	{
+	  xfree (dir);
+	  dir = canon_sysroot;
+	}
+    }
+
+  return dir;
+}
+
 /* Compute the locations of init files that GDB should source and
    return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
    there is no system gdbinit (resp. home gdbinit and local gdbinit)
@@ -370,8 +413,7 @@
   debug_file_directory = relocate_gdb_directory (DEBUGDIR,
 						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
-					GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_data_directory ();
 
 #ifdef WITH_PYTHON_PATH
   {

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Jan Kratochvil]

On Wed, 19 Sep 2012 21:55:28 +0200, Khoo Yit Phang wrote:
> +  /* Then try to find GDB_DATADIR relocated relative to gdb. */
> +  if (!dir)
> +    dir = relocate_path (gdb_program_name, GDB_DATADIR, 1,
> +			 GDB_DATADIR_RELOCATABLE);
> +
> +  /* Otherwise use GDB_DATADIR as is. */
> +  if (!dir)
> +    dir = xstrdup (GDB_DATADIR);
> +
> +  /* Canonicalize the directory.  */
> +  if (*dir)
> +    {
> +      char *canon_sysroot = lrealpath (dir);
> +
> +      if (canon_sysroot)
> +	{
> +	  xfree (dir);
> +	  dir = canon_sysroot;
> +	}
> +    }

I am not happy from the code duplication with existing function:

	char *
	relocate_gdb_directory (const char *initial, int flag)
	{
	  char *dir;

	  dir = relocate_path (gdb_program_name, initial, 1, flag);
	  if (!dir)
	    dir = xstrdup (initial);

	  /* Canonicalize the directory.  */
	  if (*dir)
	    {
	      char *canon_sysroot = lrealpath (dir);

	      if (canon_sysroot)
		{
		  xfree (dir);
		  dir = canon_sysroot;
		}
	    }

	  return dir;
	}

Also it could check for /usr/gdb/data-directory which is less likely to be
present than /usr/bin/data-directory .

Proposing the patch below instead, although I do not like it much either.


Thanks,
Jan


gdb/
2012-09-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* main.c: New variables dir and datadir.  Change GDB_DATADIR
	initialization to attempt to find gdb/data-directory/ first.

diff --git a/gdb/main.c b/gdb/main.c
index bde37f9..635c86b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -314,6 +314,7 @@ captured_main (void *data)
   int i;
   int save_auto_load;
   struct objfile *objfile;
+  char *dir, *datadir;
 
   struct cleanup *pre_stat_chain;
 
@@ -370,8 +371,23 @@ captured_main (void *data)
   debug_file_directory = relocate_gdb_directory (DEBUGDIR,
 						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
-					GDB_DATADIR_RELOCATABLE);
+  /* Attempt first to find gdb/data-directory/ if we are run still from the
+     build directory.  */
+  datadir = ldirname (BINDIR);
+  datadir = reconcat (datadir, datadir,
+		      SLASH_STRING "gdb" SLASH_STRING "data-directory", NULL);
+  dir = relocate_path (gdb_program_name, datadir, 1, 1);
+  xfree (datadir);
+  if (dir)
+    {
+      /* DATADIR could never have less components than BINDIR, therefore
+         we do not need to check it DIR is not "" like
+         relocate_gdb_directory has to.  */
+      gdb_datadir = lrealpath (dir);
+      xfree (dir);
+    }
+  if (gdb_datadir == NULL)
+    gdb_datadir = relocate_gdb_directory (GDB_DATADIR, GDB_DATADIR_RELOCATABLE);
 
 #ifdef WITH_PYTHON_PATH
   {

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 21, 2012, at 2:31 PM, Jan Kratochvil wrote:

> I am not happy from the code duplication with existing function:

Yeah, it's not pretty, but I'm not sure how to eliminate the redundancy since if "data-directory" is found, it may have to be canonicalized as well.

> Also it could check for /usr/gdb/data-directory which is less likely to be
> present than /usr/bin/data-directory .

That does not happen, since the first part of relocate_gdb_data_directory finds BINDIR/data-directory by finding the directory containing the gdb binary, then appending "data-directory" to it. But in your patch:

> +     build directory.  */
> +  datadir = ldirname (BINDIR);
> +  datadir = reconcat (datadir, datadir,
> +		      SLASH_STRING "gdb" SLASH_STRING "data-directory", NULL);
> +  dir = relocate_path (gdb_program_name, datadir, 1, 1);

Doesn't this have a possibility of finding /usr/gdb/data-directory?

Yit
September 21, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Jan Kratochvil]

On Fri, 21 Sep 2012 21:05:36 +0200, Khoo Yit Phang wrote:
> On Sep 21, 2012, at 2:31 PM, Jan Kratochvil wrote:
> > Also it could check for /usr/gdb/data-directory which is less likely to be
> > present than /usr/bin/data-directory .
> 
> That does not happen, since the first part of relocate_gdb_data_directory finds BINDIR/data-directory by finding the directory containing the gdb binary, then appending "data-directory" to it. But in your patch:
> 
> > +     build directory.  */
> > +  datadir = ldirname (BINDIR);
> > +  datadir = reconcat (datadir, datadir,
> > +		      SLASH_STRING "gdb" SLASH_STRING "data-directory", NULL);
> > +  dir = relocate_path (gdb_program_name, datadir, 1, 1);
> 
> Doesn't this have a possibility of finding /usr/gdb/data-directory?

Your patch may find /usr/bin/data-directory, my patch may find
/usr/gdb/data-directory, I find the latter as a less possibly existing my
mistake.  But maybe it does not matter much.


Jan

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi Jan,

On Sep 22, 2012, at 7:08 AM, Jan Kratochvil wrote:

> On Fri, 21 Sep 2012 21:05:36 +0200, Khoo Yit Phang wrote:
>> On Sep 21, 2012, at 2:31 PM, Jan Kratochvil wrote:
>>> Also it could check for /usr/gdb/data-directory which is less likely to be
>>> present than /usr/bin/data-directory .
>> 
>> That does not happen, since the first part of relocate_gdb_data_directory finds BINDIR/data-directory by finding the directory containing the gdb binary, then appending "data-directory" to it. But in your patch:
>> 
>>> +     build directory.  */
>>> +  datadir = ldirname (BINDIR);
>>> +  datadir = reconcat (datadir, datadir,
>>> +		      SLASH_STRING "gdb" SLASH_STRING "data-directory", NULL);
>>> +  dir = relocate_path (gdb_program_name, datadir, 1, 1);
>> 
>> Doesn't this have a possibility of finding /usr/gdb/data-directory?
> 
> Your patch may find /usr/bin/data-directory, my patch may find
> /usr/gdb/data-directory, I find the latter as a less possibly existing my
> mistake.  But maybe it does not matter much.

Personally, I'm not fond of either my patch or yours, since they hard code parts of the build directory into the binary (whether "data-directory" or "gdb/data-directory"). In either case, they can lead to weird, difficult-to-understand behavior (possibly even dangerous) in the off chance that one those directory exist. I'd rather leave it out.

Yit
September 22, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> Personally, I'm not fond of either my patch or yours, since they hard
> code parts of the build directory into the binary (whether
> "data-directory" or "gdb/data-directory"). In either case, they can
> lead to weird, difficult-to-understand behavior (possibly even
> dangerous) in the off chance that one those directory exist. I'd
> rather leave it out.

Well, for my part, I cannot wait for this patch to go in, because
I've grown accustomed to being able to just run the GDB from the
build directory. Having to install it now is an important downer
for me.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 3:29 AM, Joel Brobecker wrote:

>> Personally, I'm not fond of either my patch or yours, since they hard
>> code parts of the build directory into the binary (whether
>> "data-directory" or "gdb/data-directory"). In either case, they can
>> lead to weird, difficult-to-understand behavior (possibly even
>> dangerous) in the off chance that one those directory exist. I'd
>> rather leave it out.
> 
> Well, for my part, I cannot wait for this patch to go in, because
> I've grown accustomed to being able to just run the GDB from the
> build directory. Having to install it now is an important downer
> for me.

How about this solution, instead of patching the gdb binary, I can install a shell script into the build directory that contains:

#!/bin/sh
exec $BUILDDIR/gdb/gdb -data-directory $BUILDDIR/gdb/data-directory "$@"

and call this script "gdb-local" (or alternatively, call this script "gdb" and the rename the actual binary to "gdb.exe" in the build directory).

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Mon, 24 Sep 2012 09:14:21 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> > Well, for my part, I cannot wait for this patch to go in, because
> > I've grown accustomed to being able to just run the GDB from the
> > build directory. Having to install it now is an important downer
> > for me.
> 
> How about this solution, instead of patching the gdb binary, I can install a shell script into the build directory that contains:
> 
> #!/bin/sh
> exec $BUILDDIR/gdb/gdb -data-directory $BUILDDIR/gdb/data-directory "$@"
> 
> and call this script "gdb-local" (or alternatively, call this script "gdb" and the rename the actual binary to "gdb.exe" in the build directory).

Please don't.  Invoking GDB from the build directory should "just
work".

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 10:24 AM, Eli Zaretskii wrote:

>> From: Khoo Yit Phang <khooyp@cs.umd.edu>
>> 
>> How about this solution, instead of patching the gdb binary, I can install a shell script into the build directory that contains:
>> 
>> #!/bin/sh
>> exec $BUILDDIR/gdb/gdb -data-directory $BUILDDIR/gdb/data-directory "$@"
>> 
>> and call this script "gdb-local" (or alternatively, call this script "gdb" and the rename the actual binary to "gdb.exe" in the build directory).
> 
> Please don't.  Invoking GDB from the build directory should "just
> work".

What do you mean? If we call the shell script "gdb" (only in the build-directory, to clarify; it will not be installed to /usr/bin), then it will "just work" in almost all cases, except when running gdb on gdb. If we patch the gdb binary and incur the risk that a stray data-directory will lead to bugs. The first case is only slightly less convenient for GDB developers, and the second case is permanent to GDB users when installed.

Even before my Python patch, it didn't always "just work", since gdb was picking up the wrong data-directory, and any updates to data-directory (XML signals or Python scripts) would have been missed.

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Mon, 24 Sep 2012 10:37:17 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,
>  brobecker@adacore.com,
>  jan.kratochvil@redhat.com,
>  gdb-patches@sourceware.org
> 
> > Please don't.  Invoking GDB from the build directory should "just
> > work".
> 
> What do you mean? If we call the shell script "gdb" (only in the build-directory, to clarify; it will not be installed to /usr/bin), then it will "just work" in almost all cases, except when running gdb on gdb.

First, running gdb on gdb is an important use case.

Second, there are systems (like MS-Windows) which cannot run Unix
shell scripts natively.

Third, why should I trust random shell scripts that come with the
distribution?

Forth, having a shell script that shadows a binary leads to confusion
and aggravation if the user is not aware of that dichotomy.

There are probably more reasons why.

> If we patch the gdb binary and incur the risk that a stray data-directory will lead to bugs.

I don't see how is this different from risk of running a stray shell
script.

> Even before my Python patch, it didn't always "just work", since gdb was picking up the wrong data-directory, and any updates to data-directory (XML signals or Python scripts) would have been missed.

I'm okay with fixing that.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 10:51 AM, Eli Zaretskii wrote:

>> From: Khoo Yit Phang <khooyp@cs.umd.edu>
>> Date: Mon, 24 Sep 2012 10:37:17 -0400
>> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,
>> brobecker@adacore.com,
>> jan.kratochvil@redhat.com,
>> gdb-patches@sourceware.org
>> 
>>> Please don't.  Invoking GDB from the build directory should "just
>>> work".
>> 
>> What do you mean? If we call the shell script "gdb" (only in the build-directory, to clarify; it will not be installed to /usr/bin), then it will "just work" in almost all cases, except when running gdb on gdb.
> 
> First, running gdb on gdb is an important use case.

Yes, and instead of running "gdb gdb/gdb", you'd run "gdb gdb/gdb-the-real-binary", which I think is trivial for GDB developers.

> Second, there are systems (like MS-Windows) which cannot run Unix
> shell scripts natively.

That's easy to fix, write the redirector as a C program and compile it at the same time gdb-the-real-binary is compiled.

> Third, why should I trust random shell scripts that come with the
> distribution?

Why would you trust any random binary, including gdb, that gets installed in the build directory? In my suggestion, the redirector will *never* be installed to /usr/bin.

> Forth, having a shell script that shadows a binary leads to confusion
> and aggravation if the user is not aware of that dichotomy.

No *user* should ever see the redirector once GDB has been installed. It's only ever in the build directory for GDB developers to run GDB from the build directory.

> There are probably more reasons why.
> 
>> If we patch the gdb binary and incur the risk that a stray data-directory will lead to bugs.
> 
> I don't see how is this different from risk of running a stray shell
> script.

Again, the redirector will never be installed to /usr/bin, whereas gdb-the-real-binary is always installed.

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 11:00 AM, Khoo Yit Phang wrote:

> On Sep 24, 2012, at 10:51 AM, Eli Zaretskii wrote:
> 
>>> From: Khoo Yit Phang <khooyp@cs.umd.edu>
>> 
>> First, running gdb on gdb is an important use case.
> 
> Yes, and instead of running "gdb gdb/gdb", you'd run "gdb gdb/gdb-the-real-binary", which I think is trivial for GDB developers.
> 
>> Second, there are systems (like MS-Windows) which cannot run Unix
>> shell scripts natively.
> 
> That's easy to fix, write the redirector as a C program and compile it at the same time gdb-the-real-binary is compiled.

Come to think of it, with GDB's follow-exec-mode, making the redirector a C program installed as "gdb/gdb" that exec's "gdb/gdb-the-real-binary" should also allow running "gdb gdb/gdb" to work, shouldn't it?

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Mon, 24 Sep 2012 11:27:20 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  brobecker@adacore.com,
>  jan.kratochvil@redhat.com,
>  gdb-patches@sourceware.org
> 
> Come to think of it, with GDB's follow-exec-mode, making the redirector a C program installed as "gdb/gdb" that exec's "gdb/gdb-the-real-binary" should also allow running "gdb gdb/gdb" to work, shouldn't it?

Only if follow-exec-mode works well (AFAIK, it doesn't on Windows).
And even then, the user could find it surprising and unexpected.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> How about this solution, instead of patching the gdb binary, I can
> install a shell script into the build directory that contains:

In addition to Eli's comments, a shell script isn't debuggable.

I know we are walking on thin ice with this data-directory issue,
but I think it's worth our trouble in this case. The side-effect
of your _gdb patch is something that's really important to me;
that's why I am pushing a little harder than usual.

As far as I could tell, Jan and yourself were discussing two patches
that should work. You like neither of them, but short of having
a solution that either of you likes, why not just commit one of them?
As long as we take the build data-directory as a fallback over a non-
existant standard (configured/possibly relocated) data-directory,
I don't see a problem.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 10:59 AM, Joel Brobecker wrote:

>> How about this solution, instead of patching the gdb binary, I can
>> install a shell script into the build directory that contains:
> 
> In addition to Eli's comments, a shell script isn't debuggable.
> 
> I know we are walking on thin ice with this data-directory issue,
> but I think it's worth our trouble in this case. The side-effect
> of your _gdb patch is something that's really important to me;
> that's why I am pushing a little harder than usual.
> 
> As far as I could tell, Jan and yourself were discussing two patches
> that should work. You like neither of them, but short of having
> a solution that either of you likes, why not just commit one of them?
> As long as we take the build data-directory as a fallback over a non-
> existant standard (configured/possibly relocated) data-directory,
> I don't see a problem.

I'd just like to point out any issues that may arise in the future and offer other solutions; but I'm willing to commit either patch. Which one would you prefer?

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> Date: Mon, 24 Sep 2012 16:59:10 +0200
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Jan Kratochvil <jan.kratochvil@redhat.com>,	GDB Patches <gdb-patches@sourceware.org>
> 
> As far as I could tell, Jan and yourself were discussing two patches
> that should work. You like neither of them, but short of having
> a solution that either of you likes, why not just commit one of them?
> As long as we take the build data-directory as a fallback over a non-
> existant standard (configured/possibly relocated) data-directory,
> I don't see a problem.

Agreed.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 10:59 AM, Joel Brobecker wrote:

> As long as we take the build data-directory as a fallback over a non-
> existant standard (configured/possibly relocated) data-directory,
> I don't see a problem.

Oh, I missed this, the way it works with either patch, the build data-directory is *not* a fallback, it *overrides* the standard data-directory. That's the only way it can work if the standard data-directory exists and is out of date.

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> > As long as we take the build data-directory as a fallback over a non-
> > existant standard (configured/possibly relocated) data-directory,
> > I don't see a problem.
> 
> Oh, I missed this, the way it works with either patch, the build
> data-directory is *not* a fallback, it *overrides* the standard
> data-directory. That's the only way it can work if the standard
> data-directory exists and is out of date.

Overriding would actually work better for me, but I didn't realize that
this was what was discussed. I can live with either approaches, as
either have pros and cons, depending on the situation that a developer
might face. Personally, the debuggers I work on are never installed,
so having it be a fallback would be just find.

Alternatively, we could think of having a routine that detects that
the GDB binary is uninstalled, and relocate all the various directories
differently. But that might be a bigger change that you'd want to, and
I'm not sure this is something that the community might want (because
it would add useless code for most users, only used by developers).

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 11:26 AM, Joel Brobecker wrote:

> Alternatively, we could think of having a routine that detects that
> the GDB binary is uninstalled, and relocate all the various directories
> differently. But that might be a bigger change that you'd want to, and
> I'm not sure this is something that the community might want (because
> it would add useless code for most users, only used by developers).

A simpler alternative would be to detect if the gdb_program_name == $BUILDDIR/gdb/gdb, then only look for $BUILDDIR/gdb/data-directory. It would still hard code parts of the build directory into the binary (i.e., make sure you don't have any embarrassing/privacy-leaking paths in your home directory structure), but would much lessen the risk of a stray data-directory. What do you think?

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Khoo Yit Phang]

Hi,

On Sep 24, 2012, at 12:09 PM, Khoo Yit Phang wrote:

> Hi,
> 
> On Sep 24, 2012, at 11:26 AM, Joel Brobecker wrote:
> 
>> Alternatively, we could think of having a routine that detects that
>> the GDB binary is uninstalled, and relocate all the various directories
>> differently. But that might be a bigger change that you'd want to, and
>> I'm not sure this is something that the community might want (because
>> it would add useless code for most users, only used by developers).
> 
> A simpler alternative would be to detect if the gdb_program_name == $BUILDDIR/gdb/gdb, then only look for $BUILDDIR/gdb/data-directory. It would still hard code parts of the build directory into the binary (i.e., make sure you don't have any embarrassing/privacy-leaking paths in your home directory structure), but would much lessen the risk of a stray data-directory. What do you think?

Here's yet another alternative: gdb already tries to locate data-directory in $BUILDDIR/share/gdb as part of the relocation logic; we can install a symlink there that points to $BUILDDIR/gdb/data-directory, or make a copy for platforms that don't support symlinks.

Yit
September 24, 2012

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> Here's yet another alternative: gdb already tries to locate
> data-directory in $BUILDDIR/share/gdb as part of the relocation logic;
> we can install a symlink there that points to
> $BUILDDIR/gdb/data-directory, or make a copy for platforms that don't
> support symlinks.

Sounds like an interesting idea. We could even do a local install
of the share directory with $BUILDDIR as the prefix, to avoid
duplicating the code, if that helps. But people might complain that
this steps is unnecessary for those who always do an install after
build. I think it's sufficiently negligible to be OK, personally.

-- 
Joel

[PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

Hi,

On Sep 24, 2012, at 1:03 PM, Joel Brobecker wrote:

>> Here's yet another alternative: gdb already tries to locate
>> data-directory in $BUILDDIR/share/gdb as part of the relocation logic;
>> we can install a symlink there that points to
>> $BUILDDIR/gdb/data-directory, or make a copy for platforms that don't
>> support symlinks.
> 
> Sounds like an interesting idea. We could even do a local install
> of the share directory with $BUILDDIR as the prefix, to avoid
> duplicating the code, if that helps. But people might complain that
> this steps is unnecessary for those who always do an install after
> build. I think it's sufficiently negligible to be OK, personally.

Here's a first attempt at this. It's quite simple, but it assumes that $GDB_DATADIR and/or $bindir aren't too unusual.

Yit
September 24, 2012


[-- Attachment #2: builddir-data-directory.txt --]
[-- Type: text/plain, Size: 1010 bytes --]

gdb/ChangeLog

2012-09-24  Khoo Yit Phang  <khooyp@cs.umd.edu>

	* data-directory/Makefile.in (all): Also install data-directory
	into the build directory at the location computed by
	relocate_gdb_directory when gdb is run from the build directory.

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -95,8 +95,14 @@
 	"RUNTEST=$(RUNTEST)" \
 	"RUNTESTFLAGS=$(RUNTESTFLAGS)"
 
+# Also install data-directory into the build directory at the location
+# computed by relocate_gdb_directory when gdb is run from the build directory.
 .PHONY: all
 all: stamp-syscalls stamp-python
+	@$(MAKE) $(FLAGS_TO_PASS) \
+	  "SYSCALLS_INSTALL_DIR=$(top_builddir)/../$(SYSCALLS_INSTALL_DIR:$(prefix)/%=%)" \
+	  "PYTHON_INSTALL_DIR=$(top_builddir)/../$(PYTHON_INSTALL_DIR:$(prefix)/%=%)" \
+	  install-only
 
 # For portability's sake, we need to handle systems that don't have
 # symbolic links.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

Does anyone have any objection to this approach in principle?

> 2012-09-24  Khoo Yit Phang  <khooyp@cs.umd.edu>
> 
> 	* data-directory/Makefile.in (all): Also install data-directory
> 	into the build directory at the location computed by
> 	relocate_gdb_directory when gdb is run from the build directory.

It took me a long time to understand why you had to do things the way
you do. I would have prefered a patch that just needs to call make with
a new value for "prefix" set to "$(top_build_dir)/..", rather . But it
would not work if configured with --with-gdb-datadir.

There is an issue, I think, with your patch, unfortunately: I think
it requires GNU Make, as you are using variable substitution which
I do not think is available with all flavors of make.

There are only so many options I can see:
  1. Conditionalize this feature to having GNU Make. Not sure how to do
     that, though;
  2. Compute the in-tree directory locations during the configure
     step.

Option (2) is going to be a little more work, but maybe someone has
another simpler suggestion.

> diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
> --- a/gdb/data-directory/Makefile.in
> +++ b/gdb/data-directory/Makefile.in
> @@ -95,8 +95,14 @@
>  	"RUNTEST=$(RUNTEST)" \
>  	"RUNTESTFLAGS=$(RUNTESTFLAGS)"
>  
> +# Also install data-directory into the build directory at the location
> +# computed by relocate_gdb_directory when gdb is run from the build directory.
>  .PHONY: all
>  all: stamp-syscalls stamp-python
> +	@$(MAKE) $(FLAGS_TO_PASS) \
> +	  "SYSCALLS_INSTALL_DIR=$(top_builddir)/../$(SYSCALLS_INSTALL_DIR:$(prefix)/%=%)" \
> +	  "PYTHON_INSTALL_DIR=$(top_builddir)/../$(PYTHON_INSTALL_DIR:$(prefix)/%=%)" \
> +	  install-only
>  
>  # For portability's sake, we need to handle systems that don't have
>  # symbolic links.


-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Sep 27, 2012, at 5:17 AM, Joel Brobecker wrote:

> It took me a long time to understand why you had to do things the way
> you do. I would have prefered a patch that just needs to call make with
> a new value for "prefix" set to "$(top_build_dir)/..", rather . But it
> would not work if configured with --with-gdb-datadir.
> 
> There is an issue, I think, with your patch, unfortunately: I think
> it requires GNU Make, as you are using variable substitution which
> I do not think is available with all flavors of make.
> 
> There are only so many options I can see:
>  1. Conditionalize this feature to having GNU Make. Not sure how to do
>     that, though;
>  2. Compute the in-tree directory locations during the configure
>     step.

I could rewrite it to use sed instead, which is already used in install-python and other places. Or, we could just assume that --with-gdb-datadir or --bindir are not used during configuration and install with "prefix" set to "$(top_build_dir)/..".

Yit
September 27, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Thu, Sep 27, 2012 at 2:17 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Does anyone have any objection to this approach in principle?

I'm not entirely comfortable with this yet, but I might be persuaded.

Alternative: Is there a robust enough test to determine gdb is being
run from its build directory?
If so, you could use that to change the default value of
data-directory to $obj/gdb/data-directory.

>> 2012-09-24  Khoo Yit Phang  <khooyp@cs.umd.edu>
>>
>>       * data-directory/Makefile.in (all): Also install data-directory
>>       into the build directory at the location computed by
>>       relocate_gdb_directory when gdb is run from the build directory.
>
> It took me a long time to understand why you had to do things the way
> you do. I would have prefered a patch that just needs to call make with
> a new value for "prefix" set to "$(top_build_dir)/..", rather . But it
> would not work if configured with --with-gdb-datadir.
>
> There is an issue, I think, with your patch, unfortunately: I think
> it requires GNU Make, as you are using variable substitution which
> I do not think is available with all flavors of make.
>
> There are only so many options I can see:
>   1. Conditionalize this feature to having GNU Make. Not sure how to do
>      that, though;
>   2. Compute the in-tree directory locations during the configure
>      step.
>
> Option (2) is going to be a little more work, but maybe someone has
> another simpler suggestion.
>
>> diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
>> --- a/gdb/data-directory/Makefile.in
>> +++ b/gdb/data-directory/Makefile.in
>> @@ -95,8 +95,14 @@
>>       "RUNTEST=$(RUNTEST)" \
>>       "RUNTESTFLAGS=$(RUNTESTFLAGS)"
>>
>> +# Also install data-directory into the build directory at the location
>> +# computed by relocate_gdb_directory when gdb is run from the build directory.
>>  .PHONY: all
>>  all: stamp-syscalls stamp-python
>> +     @$(MAKE) $(FLAGS_TO_PASS) \
>> +       "SYSCALLS_INSTALL_DIR=$(top_builddir)/../$(SYSCALLS_INSTALL_DIR:$(prefix)/%=%)" \
>> +       "PYTHON_INSTALL_DIR=$(top_builddir)/../$(PYTHON_INSTALL_DIR:$(prefix)/%=%)" \
>> +       install-only
>>
>>  # For portability's sake, we need to handle systems that don't have
>>  # symbolic links.
>
>
> --
> Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> > Does anyone have any objection to this approach in principle?
> 
> I'm not entirely comfortable with this yet, but I might be persuaded.

I have to admit that I have been having second thoughts as well,
trying to figure out all the implications of this change.

> Alternative: Is there a robust enough test to determine gdb is being
> run from its build directory?

I don't see a robust way to determine that we are bing run from
the build directory. I kind of see this approach as riskier than
the first one.

I've been mulling over the following two options:

  1. Go with the idea of installing the data-directory inside the build
     directory". But this assumes that the data-directory is relocatable,
     and that it is a subdirectory of the prefix. If that's not the case,
     then I think "make all" could end up installing the data-directory
     at the configured location, outside the build directory. It seems
     like an innocent thing, but in the end, I think it's bad - almost
     sneaky.  I'd probably be the first one to curse if that happened
     to me.

  2. Accept the new situation, and configure with a --prefix somewhere
     in the build directory. I can do an install the first time, and
     then as needed based on what changes have been made since the
     last install...

It's a pain in the neck, but I think I have slowly been coming to
the conclusion that it is probably best for me, and the others who
were relying on this undocumented feature, to learn to live with
the new requirement. At least we tried...

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Wed, Oct 3, 2012 at 5:08 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> > Does anyone have any objection to this approach in principle?
>>
>> I'm not entirely comfortable with this yet, but I might be persuaded.
>
> I have to admit that I have been having second thoughts as well,
> trying to figure out all the implications of this change.
>
>> Alternative: Is there a robust enough test to determine gdb is being
>> run from its build directory?
>
> I don't see a robust way to determine that we are bing run from
> the build directory. I kind of see this approach as riskier than
> the first one.
>
> I've been mulling over the following two options:
>
>   1. Go with the idea of installing the data-directory inside the build
>      directory". But this assumes that the data-directory is relocatable,
>      and that it is a subdirectory of the prefix. If that's not the case,
>      then I think "make all" could end up installing the data-directory
>      at the configured location, outside the build directory. It seems
>      like an innocent thing, but in the end, I think it's bad - almost
>      sneaky.  I'd probably be the first one to curse if that happened
>      to me.

I think(!) this "can't happen" (if I understand the patch correctly),
the installed directory will always be a subdir of $(top_builddir)/..
It may be a useless subdirectory of $(top_builddir)/.., but at least
it's in the build tree. :-)
[Again, assuming I understand the patch correctly.]

>   2. Accept the new situation, and configure with a --prefix somewhere
>      in the build directory. I can do an install the first time, and
>      then as needed based on what changes have been made since the
>      last install...

Not sure I understand, but for reference sake,
Our builds always do a "make install" into a staging area, though we
configure --prefix=/usr.

i.e. make install DESTDIR=$build/foo
[puts the entire gdb install tree in $build/foo/usr]

And, given gdb's relocatability, it "just works" (we configure with
values for --with-gdb-datadir and --with-system-gdbinit that are
relocatable).

One still can't run gdb from the build directory (without explicitly
passing --data-directory=foo), one needs to run gdb from the staging
area, but it lets us test without having to do a "make install" into
$prefix (e.g. /usr).
[Which isn't to say we don't also test the gdb installed in $prefix.]

> It's a pain in the neck, but I think I have slowly been coming to
> the conclusion that it is probably best for me, and the others who
> were relying on this undocumented feature, to learn to live with
> the new requirement. At least we tried...

I'd be ok with a short option that said "I'm running from the build
directory, deal with it." :-)
E..g,
bash$ ./gdb -b
[alas -b is taken, but I hope you get the idea]

It's typed often enough that I can see myself getting use to it,
instead of forever avoiding "./gdb
--data-directory=$(pwd)/data-directory" because it's just too much to
type (even "make install DESTDIR=/foo" is too much to type after doing
"make" each and every time).
OTOH, one can always alias it: alias bgdb="./gdb --data-directory=foo"
(but that doesn't help debugging ./gdb in emacs).

Another wild idea is to rename the gdb in the build directory as xgdb
(akin to xgcc).  One could key off that to know gdb is being run from
the build directory.

btw, Is there a use-case of yours that I'm missing?

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> I think(!) this "can't happen" (if I understand the patch correctly),
> the installed directory will always be a subdir of $(top_builddir)/..
> It may be a useless subdirectory of $(top_builddir)/.., but at least
> it's in the build tree. :-)
> [Again, assuming I understand the patch correctly.]

You might be right - I might have missed that. But the patch cannot
be applied as is, as it relies on a GNU Make feature. So we were
going to adapt it to use sed instead.

> >   2. Accept the new situation, and configure with a --prefix somewhere
> >      in the build directory. I can do an install the first time, and
> >      then as needed based on what changes have been made since the
> >      last install...
> 
> Not sure I understand, but for reference sake,
> Our builds always do a "make install" into a staging area, though we
> configure --prefix=/usr.

I finally figured out what DESTDIR is for. Almost like a chroot
kind of thing. Neat!

What I was saying is a little TMI for what we were discussing. It
was just an example of what I do to get something installed somewhere.
I could do it your way, but it's easier with a configure-time option...

> > It's a pain in the neck, but I think I have slowly been coming to
> > the conclusion that it is probably best for me, and the others who
> > were relying on this undocumented feature, to learn to live with
> > the new requirement. At least we tried...
> 
> I'd be ok with a short option that said "I'm running from the build
> directory, deal with it." :-)
> E..g,
> bash$ ./gdb -b
> [alas -b is taken, but I hope you get the idea]

I don't think that it'll be necessary. Practically speaking, it's
easier to just do that one install, and keep it until some change
makes it need an update.

> Another wild idea is to rename the gdb in the build directory as xgdb
> (akin to xgcc).  One could key off that to know gdb is being run from
> the build directory.

I think that this is opening the door for allowing GDB to execute
code without the user being aware of it. I'd rather avoid that.

> btw, Is there a use-case of yours that I'm missing?

I do not think so. It's mostly a case where I build and then test
right away using the binary in the build directory. I've been doing
that for the past 12 years, and I find it saves time, albeit only
a little. I was exploring the idea of trying to preserve this
behavior. But I think the cost might be exceeding the benefits.

Thanks for looking into this with us, though. This is not to say
that the discussion is over; just that, so far, the options suggested
don't really help enough to be worth implementing (IMO).

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 3, 2012, at 9:33 PM, Joel Brobecker wrote:

>> Another wild idea is to rename the gdb in the build directory as xgdb
>> (akin to xgcc).  One could key off that to know gdb is being run from
>> the build directory.
> 
> I think that this is opening the door for allowing GDB to execute
> code without the user being aware of it. I'd rather avoid that.

I've thought of one more option: we can compile two near copies of gdb: the standard gdb to be installed, and xgdb for debugging (never installed). The only difference for xgdb is that it has a different main() function that inserts --data-directory into the command line before calling the real main. The implementation can be something like:

// xgdb.c
int main (int argc, char *argv[]) {
	... add --data-directory $BUILDDIR to argv if it doesn't exists ...
	return the_actual_main(argc, argv);
}

#define main the_actual_main
#include "gdb.c"
// end xgdb.c

This doesn't have the issue of my previous idea of using exec, and does not rely on any quirks of relocate_gdb_directory. It does hard code the build directory into the binary, but that binary should never be installed and so does not affect users. The cost would be compiling one more source file and linking it.

Yit
October 3, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> I've thought of one more option: we can compile two near copies of
> gdb:

I have thought of that too, but Jan worked pretty hard to reduce
the amount of time required to build GDB by removing the creation
of libgdb.a, and the build of gdbtui. Each of those step was probably
taking the amount of time it takes to link GDB. I don't think
we'll agree to regress in that respect just so that I can run GDB
without having to install...

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Wed, Oct 3, 2012 at 8:40 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> I've thought of one more option: we can compile two near copies of gdb: the standard gdb to be installed, and xgdb for debugging (never installed). The only difference for xgdb is that it has a different main() function that inserts --data-directory into the command line before calling the real main.

'tis a possibility, but it's not my first choice.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Wed, Oct 3, 2012 at 6:33 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> I think(!) this "can't happen" (if I understand the patch correctly),
>> the installed directory will always be a subdir of $(top_builddir)/..
>> It may be a useless subdirectory of $(top_builddir)/.., but at least
>> it's in the build tree. :-)
>> [Again, assuming I understand the patch correctly.]
>
> You might be right - I might have missed that. But the patch cannot
> be applied as is, as it relies on a GNU Make feature. So we were
> going to adapt it to use sed instead.

Yeah, I saw the GNU Make usage.
However, that's just an implementation detail, using sed will have the
same result (right?).

>> Another wild idea is to rename the gdb in the build directory as xgdb
>> (akin to xgcc).  One could key off that to know gdb is being run from
>> the build directory.
>
> I think that this is opening the door for allowing GDB to execute
> code without the user being aware of it. I'd rather avoid that.

How so?

>> btw, Is there a use-case of yours that I'm missing?
>
> I do not think so. It's mostly a case where I build and then test
> right away using the binary in the build directory. I've been doing
> that for the past 12 years, and I find it saves time,

Ditto.

> albeit only
> a little. I was exploring the idea of trying to preserve this
> behavior. But I think the cost might be exceeding the benefits.

I too don't want to give up on this yet.

> Thanks for looking into this with us, though. This is not to say
> that the discussion is over; just that, so far, the options suggested
> don't really help enough to be worth implementing (IMO).
>
> --
> Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> > I think that this is opening the door for allowing GDB to execute
> > code without the user being aware of it. I'd rather avoid that.
> 
> How so?

Let's say: I build a debugger and install it somewhere, and then
tell my collegues: Hey, use my super-duper GDB. Then, someone hacks
into my account, set things up to put my GDB into a situation where
it will think that it's still in a build directory, and then place
some code in the datadir/python area to auto-execute some malicious
code...

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Thu, Oct 4, 2012 at 7:51 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> > I think that this is opening the door for allowing GDB to execute
>> > code without the user being aware of it. I'd rather avoid that.
>>
>> How so?
>
> Let's say: I build a debugger and install it somewhere, and then
> tell my collegues: Hey, use my super-duper GDB. Then, someone hacks
> into my account, set things up to put my GDB into a situation where
> it will think that it's still in a build directory, and then place
> some code in the datadir/python area to auto-execute some malicious
> code...

If they've hacked into your account seems like it's game over regardless.
[All sorts of nasties could be inflicted - e.g., just hack the gdb
binary directly.]

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> If they've hacked into your account seems like it's game over regardless.
> [All sorts of nasties could be inflicted - e.g., just hack the gdb
> binary directly.]

If you guys are happy with a simple detection scheme, that's good
enough for me :).

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 4, 2012, at 11:07 AM, Doug Evans wrote:

> On Thu, Oct 4, 2012 at 7:51 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>>>> I think that this is opening the door for allowing GDB to execute
>>>> code without the user being aware of it. I'd rather avoid that.
>>> 
>>> How so?
>> 
>> Let's say: I build a debugger and install it somewhere, and then
>> tell my collegues: Hey, use my super-duper GDB. Then, someone hacks
>> into my account, set things up to put my GDB into a situation where
>> it will think that it's still in a build directory, and then place
>> some code in the datadir/python area to auto-execute some malicious
>> code...
> 
> If they've hacked into your account seems like it's game over regardless.
> [All sorts of nasties could be inflicted - e.g., just hack the gdb
> binary directly.]

I think the bigger issue is that $BUILDDIR/gdb/data-directory overrides the standard data-directory. If we detect run-from-builddir based on the presence of other files/directories, and some other application happens to use the same files/directories, then the user is basically stuck with either a non-working gdb (sans -data-directory) or having to uninstall that other application.

I prefer just installing data-directory into $BUILDDIR/share/data-directory, which is simple and works as long as gdb isn't configured with --bindir, --exec-prefix, and/or --with-gdb-datadir that are too unusual. It should cover most purposes of running gdb from the build directory, since I don't see much reason to change --bindir, --exec-prefix, and/or --with-gdb-datadir if gdb won't be installed, unless for testing those flags, in which case gdb will likely have to be installed anyways. Does anyone currently have a use case I'm missing?

I'm okay with Joel's idea of detecting based on the gdb binary name, i.e., we can link/copy another binary named "gdb-dev" (or something easy to Google) to gdb, and load the build-local data directory when it is run. This can't suffer from inadvertent collisions with other applications, and it's fairly obvious to users that they are not running the standard gdb since they have to type it out, and us GDB developers will have to run "gdb/gdb-dev gdb/gdb" to debug gdb which is trivial. There's no need to compile/link another program, but there's a small expense of dead code when installed, and a small risk of a user sym-linking "gdb-dev" to gdb (or making a copy or using execv with argv[0] == "gdb-dev").

Yit
October 6, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Sat, 6 Oct 2012 15:02:00 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, Joel Brobecker <brobecker@adacore.com>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> I think the bigger issue is that $BUILDDIR/gdb/data-directory overrides the standard data-directory. If we detect run-from-builddir based on the presence of other files/directories, and some other application happens to use the same files/directories, then the user is basically stuck with either a non-working gdb (sans -data-directory) or having to uninstall that other application.

I see no reason to assume that we will not be able to reliably detect
a GDB build directory and to distinguish between that and other
projects.  Surely, we can find at least one or 2 files that only exist
in GDB.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 6, 2012, at 3:24 PM, Eli Zaretskii wrote:

>> From: Khoo Yit Phang <khooyp@cs.umd.edu>
>> Date: Sat, 6 Oct 2012 15:02:00 -0400
>> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, Joel Brobecker <brobecker@adacore.com>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
>> 
>> I think the bigger issue is that $BUILDDIR/gdb/data-directory overrides the standard data-directory. If we detect run-from-builddir based on the presence of other files/directories, and some other application happens to use the same files/directories, then the user is basically stuck with either a non-working gdb (sans -data-directory) or having to uninstall that other application.
> 
> I see no reason to assume that we will not be able to reliably detect
> a GDB build directory and to distinguish between that and other
> projects.  Surely, we can find at least one or 2 files that only exist
> in GDB.

But we have to make assumptions about other applications that are not under our control, that they do not use the same files, which I think is an assumption that we should avoid if possible (who knows what's out there). Of course, we can greatly minimize the risk by checking for more files and/or the contents of files. But it is possible to avoid this assumption, e.g., the other two solutions in my previous email, hence my preferences.

Yit
October 6, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Sat, 6 Oct 2012 15:36:13 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,
>  dje@google.com,
>  brobecker@adacore.com,
>  jan.kratochvil@redhat.com,
>  gdb-patches@sourceware.org
> 
> On Oct 6, 2012, at 3:24 PM, Eli Zaretskii wrote:
> 
> >> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> >> Date: Sat, 6 Oct 2012 15:02:00 -0400
> >> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, Joel Brobecker <brobecker@adacore.com>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
> >> 
> >> I think the bigger issue is that $BUILDDIR/gdb/data-directory overrides the standard data-directory. If we detect run-from-builddir based on the presence of other files/directories, and some other application happens to use the same files/directories, then the user is basically stuck with either a non-working gdb (sans -data-directory) or having to uninstall that other application.
> > 
> > I see no reason to assume that we will not be able to reliably detect
> > a GDB build directory and to distinguish between that and other
> > projects.  Surely, we can find at least one or 2 files that only exist
> > in GDB.
> 
> But we have to make assumptions about other applications that are not under our control, that they do not use the same files, which I think is an assumption that we should avoid if possible (who knows what's out there).

How many other packages will have a file called doc/gdb.info or
doc/gdbint.info in their build tree?  How many of them will have
something called i386-cygwin-tdep.o or mi/mi-cmds.o?  I could find
many more files whose probability to be in a non-GDB tree is exactly
zero.

I think the problem you are afraid of doesn't exist.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 6, 2012, at 4:06 PM, Eli Zaretskii wrote:

>> But we have to make assumptions about other applications that are not under our control, that they do not use the same files, which I think is an assumption that we should avoid if possible (who knows what's out there).
> 
> How many other packages will have a file called doc/gdb.info or
> doc/gdbint.info in their build tree?  How many of them will have
> something called i386-cygwin-tdep.o or mi/mi-cmds.o?  I could find
> many more files whose probability to be in a non-GDB tree is exactly
> zero.
> 
> I think the problem you are afraid of doesn't exist.

I'm simply offering alternative solutions that work regardless of whether the problem exists or not, and in some cases, is much simpler to implement. Isn't that a good thing?

Yit
October 6, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Sat, 6 Oct 2012 16:11:55 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,
>  dje@google.com,
>  brobecker@adacore.com,
>  jan.kratochvil@redhat.com,
>  gdb-patches@sourceware.org
> 
> Hi,
> 
> On Oct 6, 2012, at 4:06 PM, Eli Zaretskii wrote:
> 
> >> But we have to make assumptions about other applications that are not under our control, that they do not use the same files, which I think is an assumption that we should avoid if possible (who knows what's out there).
> > 
> > How many other packages will have a file called doc/gdb.info or
> > doc/gdbint.info in their build tree?  How many of them will have
> > something called i386-cygwin-tdep.o or mi/mi-cmds.o?  I could find
> > many more files whose probability to be in a non-GDB tree is exactly
> > zero.
> > 
> > I think the problem you are afraid of doesn't exist.
> 
> I'm simply offering alternative solutions that work regardless of whether the problem exists or not, and in some cases, is much simpler to implement. Isn't that a good thing?

I don't consider those alternatives simpler from the user POV, sorry.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 6, 2012, at 4:28 PM, Eli Zaretskii wrote:

>> On Oct 6, 2012, at 4:06 PM, Eli Zaretskii wrote:
>> 
>>>> But we have to make assumptions about other applications that are not under our control, that they do not use the same files, which I think is an assumption that we should avoid if possible (who knows what's out there).
>>> 
>>> How many other packages will have a file called doc/gdb.info or
>>> doc/gdbint.info in their build tree?  How many of them will have
>>> something called i386-cygwin-tdep.o or mi/mi-cmds.o?  I could find
>>> many more files whose probability to be in a non-GDB tree is exactly
>>> zero.
>>> 
>>> I think the problem you are afraid of doesn't exist.
>> 
>> I'm simply offering alternative solutions that work regardless of whether the problem exists or not, and in some cases, is much simpler to implement. Isn't that a good thing?
> 
> I don't consider those alternatives simpler from the user POV, sorry.

I've outlined all the trade-offs that I can think of for various solutions; can you explain what I'm missing?

Yit
October 6, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Sat, 6 Oct 2012 16:32:22 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, dje@google.com, brobecker@adacore.com, jan.kratochvil@redhat.com, gdb-patches@sourceware.org
> 
> I've outlined all the trade-offs that I can think of for various solutions; can you explain what I'm missing?

We just disagree about the balance of merits and demerits of the
alternatives, that's all.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Sat, Oct 6, 2012 at 12:02 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> I prefer just installing data-directory into $BUILDDIR/share/data-directory, which is simple and works as long as gdb isn't configured with --bindir, --exec-prefix, and/or --with-gdb-datadir that are too unusual. It should cover most purposes of running gdb from the build directory, since I don't see much reason to change --bindir, --exec-prefix, and/or --with-gdb-datadir if gdb won't be installed, unless for testing those flags, in which case gdb will likely have to be installed anyways. Does anyone currently have a use case I'm missing?

I'm not entirely comfortable with having normal makes (as in "make
all") create files outside of its build directory (and by this I mean
$obj/gdb/../anything).  [And obviously I'm not including "make
install" here. :-)]
I'd prefer passing -bd (for build directory), for example.
[Not the best name choice since -b specifies the serial port baud
rate.  -bt for build tree, and -B are alternatives.]

I could also go with a configure option to create a second gdb binary
for use in the build tree.
I would never use it, but as long as those that want it agree to
maintain it, 'tis ok with me.
Neither the testsuite, nor anything else, can depend on it - it would
just be for developer convenience sake.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Khoo Yit Phang]

Hi,

On Oct 8, 2012, at 12:33 PM, Doug Evans wrote:

> On Sat, Oct 6, 2012 at 12:02 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>> I prefer just installing data-directory into $BUILDDIR/share/data-directory, which is simple and works as long as gdb isn't configured with --bindir, --exec-prefix, and/or --with-gdb-datadir that are too unusual. It should cover most purposes of running gdb from the build directory, since I don't see much reason to change --bindir, --exec-prefix, and/or --with-gdb-datadir if gdb won't be installed, unless for testing those flags, in which case gdb will likely have to be installed anyways. Does anyone currently have a use case I'm missing?
> 
> I'm not entirely comfortable with having normal makes (as in "make
> all") create files outside of its build directory (and by this I mean
> $obj/gdb/../anything).  [And obviously I'm not including "make
> install" here. :-)]
> I'd prefer passing -bd (for build directory), for example.
> [Not the best name choice since -b specifies the serial port baud
> rate.  -bt for build tree, and -B are alternatives.]
> 
> I could also go with a configure option to create a second gdb binary
> for use in the build tree.
> I would never use it, but as long as those that want it agree to
> maintain it, 'tis ok with me.
> Neither the testsuite, nor anything else, can depend on it - it would
> just be for developer convenience sake.

What about a configure option to enable "make all" to install to $obj/gdb/../share/gdb? With ample warnings about caveats, and for developer convenience only, of course.

Yit
October 8, 2012

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Mon, Oct 8, 2012 at 1:13 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> On Oct 8, 2012, at 12:33 PM, Doug Evans wrote:
>
>> On Sat, Oct 6, 2012 at 12:02 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>>> I prefer just installing data-directory into $BUILDDIR/share/data-directory, which is simple and works as long as gdb isn't configured with --bindir, --exec-prefix, and/or --with-gdb-datadir that are too unusual. It should cover most purposes of running gdb from the build directory, since I don't see much reason to change --bindir, --exec-prefix, and/or --with-gdb-datadir if gdb won't be installed, unless for testing those flags, in which case gdb will likely have to be installed anyways. Does anyone currently have a use case I'm missing?
>>
>> I'm not entirely comfortable with having normal makes (as in "make
>> all") create files outside of its build directory (and by this I mean
>> $obj/gdb/../anything).  [And obviously I'm not including "make
>> install" here. :-)]
>> I'd prefer passing -bd (for build directory), for example.
>> [Not the best name choice since -b specifies the serial port baud
>> rate.  -bt for build tree, and -B are alternatives.]
>>
>> I could also go with a configure option to create a second gdb binary
>> for use in the build tree.
>> I would never use it, but as long as those that want it agree to
>> maintain it, 'tis ok with me.
>> Neither the testsuite, nor anything else, can depend on it - it would
>> just be for developer convenience sake.
>
> What about a configure option to enable "make all" to install to $obj/gdb/../share/gdb? With ample warnings about caveats, and for developer convenience only, of course.

It's still having "make all" install something outside of gdb's build dir.

I won't object if someone else approves, but I'll be happy to remove
it if it ever bitrots or becomes problematic. :-)

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> I'm not entirely comfortable with having normal makes (as in "make
> all") create files outside of its build directory (and by this I mean
> $obj/gdb/../anything).

Neither am I, actually.

All in all, although I was one of the first few ones to ask for
being able to run GDB from the build directory, I just feel like
we aren't able to implement in a form that would be convenient
(Eg: command-line option), or else it'd open a can of worms (installing
stuff outside of the build directory), or come with ample caveats.
Best to recognize when the cure is worse than the disease and live
with it instead...

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> Date: Mon, 8 Oct 2012 22:48:32 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,	Jan Kratochvil <jan.kratochvil@redhat.com>,	GDB Patches <gdb-patches@sourceware.org>
> 
> All in all, although I was one of the first few ones to ask for
> being able to run GDB from the build directory, I just feel like
> we aren't able to implement in a form that would be convenient
> (Eg: command-line option), or else it'd open a can of worms (installing
> stuff outside of the build directory), or come with ample caveats.
> Best to recognize when the cure is worse than the disease and live
> with it instead...

FWIW, I do NOT think the cure is worse than the disease.  At least
some cures aren't.  Again, Emacs implements this feature for many
years, so I cannot understand why GDB won't.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> Date: Wed, 3 Oct 2012 17:08:40 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>,	Jan Kratochvil <jan.kratochvil@redhat.com>,	GDB Patches <gdb-patches@sourceware.org>
> 
> > Alternative: Is there a robust enough test to determine gdb is being
> > run from its build directory?
> 
> I don't see a robust way to determine that we are bing run from
> the build directory.

Why not?  Aren't there specific directories and/or files near the GDB
executable in this case?

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Joel Brobecker]

> Why not?  Aren't there specific directories and/or files near the GDB
> executable in this case?

I have a feeling that this would open the door allowing attackers
to setup GDB to execute unwanted code if we make it easy to reproduce
the same environment and place GDB in a mode where it thinks it is
inside a build directory. What we need, I think, is a way to tie
the build to the data directory in a way that would be very very
hard to forge, like keeping a signature of one of the files in the
data directory - but for that to work, we'd need something to be
random in that file. And then exclude that file from being installed.
Once we have that, we can modify GDB to check ./data-directory/ for
the special file before setting the data-directory...

But beyond this technical aspect, I am reluctant to add a mode to GDB
that would then become useless to 99% of the people once GDB is properly
installed. I don't think that the convenience it provides is important
enough to justify it.

That being said: I am not opposed to this idea at all. If people
find a way to implement this and gets approved, that's absolutely OK
with me!

-- 
Joel

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Thu, Oct 4, 2012 at 6:49 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Why not?  Aren't there specific directories and/or files near the GDB
>> executable in this case?
>
> I have a feeling that this would open the door allowing attackers
> to setup GDB to execute unwanted code if we make it easy to reproduce
> the same environment and place GDB in a mode where it thinks it is
> inside a build directory.

auto-load safe-path isn't circumvented.
Ever done ./gdb ./gdb and got the complaint about gdb-gdb.gdb not
being loaded? :-)
[Working around that is in my ~/.gdbinit, but I still run into it from
time to time.]

Heh.  A thought occurred to me.
The default value of "auto-load safe-path" is
$debugdir:$datadir/auto-load.
Is it a bug that
./gdb --data-directory $(pwd)/data-directory ./gdb
loads data-directory/python/gdb/__init__.py
?

And do we need to augment Python's module loader to handle gdb's
auto-load safe-path?

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Doug Evans]

On Thu, Oct 4, 2012 at 7:48 AM, Doug Evans <dje@google.com> wrote:
> On Thu, Oct 4, 2012 at 6:49 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>>> Why not?  Aren't there specific directories and/or files near the GDB
>>> executable in this case?
>>
>> I have a feeling that this would open the door allowing attackers
>> to setup GDB to execute unwanted code if we make it easy to reproduce
>> the same environment and place GDB in a mode where it thinks it is
>> inside a build directory.
>
> auto-load safe-path isn't circumvented.

Hmm, I take that back, since the plan was to set a different default
value for data-directory.

I wonder if it's possible to put a solution in ~/.gdbinit.
gdb hackers already have to do something if they want gdb-gdb.gdb
auto-loaded (assuming they don't do something at configure time).
[If we need to extend the Python API a bit, that'd be ok with me.]
~/.gdbinit is processed before the command line args.

Re: [PATCH] Also install data-directory into the build directory as computed by relocate_gdb_directory

[Eli Zaretskii]

> Date: Thu, 4 Oct 2012 06:49:28 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: dje@google.com, khooyp@cs.umd.edu, jan.kratochvil@redhat.com,
> 	gdb-patches@sourceware.org
> 
> > Why not?  Aren't there specific directories and/or files near the GDB
> > executable in this case?
> 
> I have a feeling that this would open the door allowing attackers
> to setup GDB to execute unwanted code if we make it easy to reproduce
> the same environment and place GDB in a mode where it thinks it is
> inside a build directory.

Well, FWIW, Emacs uses a similar scheme (it tests for a couple of
files in specific directories around itself to decide whether it is
invoked without being installed), and I don't think anyone complained
about circumventions.

> But beyond this technical aspect, I am reluctant to add a mode to GDB
> that would then become useless to 99% of the people once GDB is properly
> installed. I don't think that the convenience it provides is important
> enough to justify it.

The question is: what's the price?  If the price is sufficiently low,
I don't see why you should be reluctant, even if you are right and 99%
of users will find this useless.  (FWIW, I almost always use GDB from
its build directory, I don't even know why.)

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Mon, 24 Sep 2012 12:45:21 -0400
> Cc: Joel Brobecker <brobecker@adacore.com>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> > A simpler alternative would be to detect if the gdb_program_name == $BUILDDIR/gdb/gdb, then only look for $BUILDDIR/gdb/data-directory. It would still hard code parts of the build directory into the binary (i.e., make sure you don't have any embarrassing/privacy-leaking paths in your home directory structure), but would much lessen the risk of a stray data-directory. What do you think?
> 
> Here's yet another alternative: gdb already tries to locate data-directory in $BUILDDIR/share/gdb as part of the relocation logic; we can install a symlink there that points to $BUILDDIR/gdb/data-directory, or make a copy for platforms that don't support symlinks.

I liked the first of these two better.  It is simpler.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> > > A simpler alternative would be to detect if the gdb_program_name
> > > == $BUILDDIR/gdb/gdb, then only look for
> > > $BUILDDIR/gdb/data-directory. It would still hard code parts of
> > > the build directory into the binary (i.e., make sure you don't
> > > have any embarrassing/privacy-leaking paths in your home directory
> > > structure), but would much lessen the risk of a stray
> > > data-directory. What do you think?
> > 
> > Here's yet another alternative: gdb already tries to locate
> > data-directory in $BUILDDIR/share/gdb as part of the relocation
> > logic; we can install a symlink there that points to
> > $BUILDDIR/gdb/data-directory, or make a copy for platforms that
> > don't support symlinks.
> 
> I liked the first of these two better.  It is simpler.

I am not sure it is simpler, because I don't think that the GDB binary
knows what the $BUILDDIR was. I am also wondering whether we might
activate the in-true use feature by accident too. I like the second
suggestion because it's very simple (as you could see from the patch),
and avoids touching the behavior in the installed-case. In other words,
our requirement does not affect the behavior at the end-user level.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> Date: Mon, 24 Sep 2012 22:49:31 +0200
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, jan.kratochvil@redhat.com,
> 	gdb-patches@sourceware.org
> 
> > > > A simpler alternative would be to detect if the gdb_program_name
> > > > == $BUILDDIR/gdb/gdb, then only look for
> > > > $BUILDDIR/gdb/data-directory. It would still hard code parts of
> > > > the build directory into the binary (i.e., make sure you don't
> > > > have any embarrassing/privacy-leaking paths in your home directory
> > > > structure), but would much lessen the risk of a stray
> > > > data-directory. What do you think?
> > > 
> > > Here's yet another alternative: gdb already tries to locate
> > > data-directory in $BUILDDIR/share/gdb as part of the relocation
> > > logic; we can install a symlink there that points to
> > > $BUILDDIR/gdb/data-directory, or make a copy for platforms that
> > > don't support symlinks.
> > 
> > I liked the first of these two better.  It is simpler.
> 
> I am not sure it is simpler, because I don't think that the GDB binary
> knows what the $BUILDDIR was.

What's wrong with "../data-directory" relative to the directory from
which 'gdb' was invoked?

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> What's wrong with "../data-directory" relative to the directory from
> which 'gdb' was invoked?

I my opinion, it opens a very obvious window for running something
the user didn't mean to run.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> Date: Mon, 24 Sep 2012 23:34:57 +0200
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: khooyp@cs.umd.edu, jan.kratochvil@redhat.com,
> 	gdb-patches@sourceware.org
> 
> > What's wrong with "../data-directory" relative to the directory from
> > which 'gdb' was invoked?
> 
> I my opinion, it opens a very obvious window for running something
> the user didn't mean to run.

I don't see how.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> > I my opinion, it opens a very obvious window for running something
> > the user didn't mean to run.
> 
> I don't see how.

There are Python files in inside one of its directories, and they can
get executed when starting up. Take a look at the __init__.py files in
gdb/python/lib/gdb and gdb/python/lib/gdb/command, for instance.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> Date: Tue, 25 Sep 2012 08:35:18 +0200
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: khooyp@cs.umd.edu, jan.kratochvil@redhat.com,
> 	gdb-patches@sourceware.org
> 
> > > I my opinion, it opens a very obvious window for running something
> > > the user didn't mean to run.
> > 
> > I don't see how.
> 
> There are Python files in inside one of its directories, and they can
> get executed when starting up. Take a look at the __init__.py files in
> gdb/python/lib/gdb and gdb/python/lib/gdb/command, for instance.

That's not what I meant.  I don't see how using absolute file names is
safer than using file names computed from relative file names.

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Joel Brobecker]

> That's not what I meant.  I don't see how using absolute file names is
> safer than using file names computed from relative file names.

You're right - it's not a great deal safer than with relative file
names. At least there is only one directory from which GDB is
susceptible to attack. But the possibility still exists, and probably
a simple "string" program can find the right location.

The latest proposal of installing a copy of the share directory
at the location where GDB would look for them is fairly clever
that way, since it does not open a new door. And since it was proposed,
I know I've seen this approach used in other projects as well.

-- 
Joel

Re: [PATCH 2/2] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Eli Zaretskii]

> From: Khoo Yit Phang <khooyp@cs.umd.edu>
> Date: Mon, 24 Sep 2012 12:09:34 -0400
> Cc: Khoo Yit Phang <khooyp@cs.umd.edu>, Jan Kratochvil <jan.kratochvil@redhat.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> A simpler alternative would be to detect if the gdb_program_name == $BUILDDIR/gdb/gdb, then only look for $BUILDDIR/gdb/data-directory. It would still hard code parts of the build directory into the binary (i.e., make sure you don't have any embarrassing/privacy-leaking paths in your home directory structure), but would much lessen the risk of a stray data-directory. What do you think?

Sounds fine to me.