Hard constraint on maximum number of fields in ParamReader about quip

[jameskermode]

I have had to revert the increase of this parameter in commit 37e5b003 as it was leading to segmentation faults. Here's a backtrace in case anyone is enthused to dig into this:

gdb) bt
#0  0x00000000009d757e in paramreader_module::param_parse_value (entry=<error reading variable: Cannot access memory at address 0x7fffff5f6968>,
   key=<error reading variable: Cannot access memory at address 0x7fffff5f6960>,
   _key=<error reading variable: Cannot access memory at address 0x7fffff5f695c>) at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:842
#1  0x00000000009e02a3 in paramreader_module::param_register_main (dict=..., key='verbosity', value='NORMAL', n=1, param_type=3,
   help_string='verbosity level', int_target=<error reading variable: Cannot access memory at address 0x0>,
   real_target=<error reading variable: Cannot access memory at address 0x0>, char_target='\000' <repeats 10240 times>,
   logical_target=<error reading variable: Cannot access memory at address 0x0>,
   int_target_array=<error reading variable: Cannot resolve DW_OP_push_object_address for a missing object>,
   real_target_array=<error reading variable: Cannot resolve DW_OP_push_object_address for a missing object>,
   has_value_target=<error reading variable: Cannot access memory at address 0x0>, _key=9, _value=6, _help_string=15, _char_target=10240)
   at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:327
#2  0x00000000009e0bc5 in paramreader_module::param_register_single_string (dict=..., key='verbosity', value='NORMAL',
   char_target='\000' <repeats 10240 times>, help_string='verbosity level',
   has_value_target=<error reading variable: Cannot access memory at address 0x0>, _key=9, _value=6, _char_target=10240, _help_string=15)
   at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:200
#3  0x0000000000408e74 in quip () at /home/eng/essswb/QUIP/src/Programs/quip.f95:133
#4  0x0000000000414759 in main (argc=13, argv=0x7fffffffd4a1) at /home/eng/essswb/QUIP/src/Programs/quip.f95:35
#5  0x00007ffff5757c36 in __libc_start_main () from /lib64/libc.so.6
#6  0x0000000000406e39 in _start () at ../sysdeps/x86_64/elf/start.S:113

[gabor1]

can we find a compromise number? It was hitting the CG-GAP stuff.

On 26 Jun 2015, at 15:23, James Kermode [email protected] wrote:

I have had to revert the increase of this parameter in commit 37e5b00 as it was leading to segmentation faults. Here's a backtrace in case anyone is enthused to dig into this:

gdb) bt
#0 0x00000000009d757e in paramreader_module::param_parse_value (entry=,
key=,
_key=) at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:842
#1 0x00000000009e02a3 in paramreader_module::param_register_main (dict=..., key='verbosity', value='NORMAL', n=1, param_type=3,
help_string='verbosity level', int_target=,
real_target=, char_target='\000' ,
logical_target=,
int_target_array=,
real_target_array=,
has_value_target=, _key=9, _value=6, _help_string=15, _char_target=10240)
at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:327
#2 0x00000000009e0bc5 in paramreader_module::param_register_single_string (dict=..., key='verbosity', value='NORMAL',
char_target='\000' , help_string='verbosity level',
has_value_target=, _key=9, _value=6, _char_target=10240, _help_string=15)
at /home/eng/essswb/QUIP/src/libAtoms/ParamReader.f95:200
#3 0x0000000000408e74 in quip () at /home/eng/essswb/QUIP/src/Programs/quip.f95:133
#4 0x0000000000414759 in main (argc=13, argv=0x7fffffffd4a1) at /home/eng/essswb/QUIP/src/Programs/quip.f95:35
#5 0x00007ffff5757c36 in __libc_start_main () from /lib64/libc.so.6
#6 0x0000000000406e39 in _start () at ../sysdeps/x86_64/elf/start.S:113

—
Reply to this email directly or view it on GitHub.

-- Gábor

||
|| Dr Gábor Csányi
|| Computational Mechanics, Engineering Laboratory
|| Pembroke College
|| University of Cambridge
||
|| PGP Public key at http://pgp.mit.edu ID: C88E9A49
|| Tel: [phone]
||

[tdaff]

This is causing segfauls again since it got bumped up to 300. Many Linuxes default to 8 MB of stack so ParamReader is trying to eat 6 MB of that. I think making it fully dynamic might be a good idea, but to mitigate the segfaults maybe it can be forced onto the heap for now? I'm not sure what the best way to do that is, anyone have any preferences? I think allocatable, save and -fmax-stack-var-size=... should work, any other ideas?

[albapa]

allocatable, save might be best, as it will work with other compilers, too.

On 22 January 2016 at 16:31, Tom Daff [email protected] wrote:

This is causing segfauls again since it got bumped up to 300. Many Linuxes
default to 8 MB of stack so ParamReader is trying to eat 6 MB of that. I
think making it fully dynamic might be a good idea, but to mitigate the
segfaults maybe it can be forced onto the heap for now? I'm not sure what
the best way to do that is, anyone have any preferences? I think
allocatable, save and -fmax-stack-var-size=... should work, any other
ideas?

—
Reply to this email directly or view it on GitHub
#8 (comment).

Dr Albert Bartok-Partay
Leverhulme Early Career Fellow

Department of Engineering, University of Cambridge, Cambridge, CB21PZ
Magdalene College, Cambridge, CB30AG

T: [phone]
W: http://www.bartokpartay.com
E: [email protected]
PGP public key: http://pgp.mit.edu (ID:FC646FB1)

[jameskermode]

The current value of 300 is causing seg faults for me on Mac OS X.

Is anyone working on implementing the allocatable, save fix?

[albapa]

I was going to do that as it was me who bumped it to 300!

On 26 January 2016 at 11:11, James Kermode [email protected] wrote:

The current value of 300 is causing seg faults for me on Mac OS X.

Is anyone working on implementing the allocatable, save fix?

—
Reply to this email directly or view it on GitHub
#8 (comment).

[tdaff]

For a quick hacky fix, this was working for me; I didn't push or anything though.

diff --git a/src/libAtoms/ParamReader.f95 b/src/libAtoms/ParamReader.f95
index 76613b8..ba1ffb1 100644
--- a/src/libAtoms/ParamReader.f95
+++ b/src/libAtoms/ParamReader.f95
@@ -351,7 +351,7 @@ module paramreader_module

       character(len=STRING_LENGTH) :: field
       integer equal_pos
-      character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: final_fields
+      character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: final_fields
       character(len=STRING_LENGTH) :: key, value
       integer :: i, num_pairs
       type(ParamEntry) :: entry
@@ -844,7 +844,7 @@ module paramreader_module
       character(len=*), optional :: key
       logical :: status

-      character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: fields
+      character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: fields
       integer :: num_fields, j

       if (entry%param_type == PARAM_NO_VALUE .or. &

[albapa]

Fix pushed!

On 26 January 2016 at 11:13, Tom Daff [email protected] wrote:

For a quick hacky fix, this was working for me; I didn't push or anything
though.

diff --git a/src/libAtoms/ParamReader.f95 b/src/libAtoms/ParamReader.f95
index 76613b8..ba1ffb1 100644
--- a/src/libAtoms/ParamReader.f95
+++ b/src/libAtoms/ParamReader.f95
@@ -351,7 +351,7 @@ module paramreader_module

   character(len=STRING_LENGTH) :: field
   integer equal_pos

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: final_fields
  

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: final_fields
  

  character(len=STRING_LENGTH) :: key, value
  integer :: i, num_pairs
  type(ParamEntry) :: entry
  @@ -844,7 +844,7 @@ module paramreader_module
  character(len=*), optional :: key
  logical :: status

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: fields
  

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: fields
  

  integer :: num_fields, j

  if (entry%param_type == PARAM_NO_VALUE .or. &

—
Reply to this email directly or view it on GitHub
#8 (comment).

[gabor1]

Is is just that the “save” option puts it onto the heap instead of the stack?

On 26 Jan 2016, at 12:13, Tom Daff [email protected] wrote:

For a quick hacky fix, this was working for me; I didn't push or anything though.

diff --git a/src/libAtoms/ParamReader.f95 b/src/libAtoms/ParamReader.f95
index 76613b8..ba1ffb1 100644
--- a/src/libAtoms/ParamReader.f95
+++ b/src/libAtoms/ParamReader.f95
@@ -351,7 +351,7 @@ module paramreader_module

   character(len=STRING_LENGTH) :: field
   integer equal_pos

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: final_fields
  

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: final_fields
  

  character(len=STRING_LENGTH) :: key, value
  integer :: i, num_pairs
  type(ParamEntry) :: entry
  @@ -844,7 +844,7 @@ module paramreader_module
  character(len=*), optional :: key
  logical :: status

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: fields
  

•  character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: fields
  

  integer :: num_fields, j

  if (entry%param_type == PARAM_NO_VALUE .or. &
  —
  Reply to this email directly or view it on GitHub #8 (comment).

[albapa]

I fixed it properly, i.e. it's dynamically allocated and deallocated.

On 26 January 2016 at 11:36, gabor1 [email protected] wrote:

Is is just that the “save” option puts it onto the heap instead of the
stack?

On 26 Jan 2016, at 12:13, Tom Daff [email protected] wrote:

For a quick hacky fix, this was working for me; I didn't push or
anything though.

diff --git a/src/libAtoms/ParamReader.f95 b/src/libAtoms/ParamReader.f95
index 76613b8..ba1ffb1 100644
--- a/src/libAtoms/ParamReader.f95
+++ b/src/libAtoms/ParamReader.f95
@@ -351,7 +351,7 @@ module paramreader_module

character(len=STRING_LENGTH) :: field
integer equal_pos

• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: final_fields
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save ::
  final_fields
  character(len=STRING_LENGTH) :: key, value
  integer :: i, num_pairs
  type(ParamEntry) :: entry
  @@ -844,7 +844,7 @@ module paramreader_module
  character(len=*), optional :: key
  logical :: status
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: fields
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: fields
  integer :: num_fields, j

if (entry%param_type == PARAM_NO_VALUE .or. &
—
Reply to this email directly or view it on GitHub <
https://github.com/libAtoms/QUIP/issues/8#issuecomment-174960155>.

—
Reply to this email directly or view it on GitHub
#8 (comment).

[albapa]

But to answer your question, the specification doesn't say anything about
that, but I guess most compilers will implement it this way. In any case, I
hope it won't be a problem in the future.

On 26 January 2016 at 11:36, gabor1 [email protected] wrote:

Is is just that the “save” option puts it onto the heap instead of the
stack?

On 26 Jan 2016, at 12:13, Tom Daff [email protected] wrote:

For a quick hacky fix, this was working for me; I didn't push or
anything though.

diff --git a/src/libAtoms/ParamReader.f95 b/src/libAtoms/ParamReader.f95
index 76613b8..ba1ffb1 100644
--- a/src/libAtoms/ParamReader.f95
+++ b/src/libAtoms/ParamReader.f95
@@ -351,7 +351,7 @@ module paramreader_module

character(len=STRING_LENGTH) :: field
integer equal_pos

• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: final_fields
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save ::
  final_fields
  character(len=STRING_LENGTH) :: key, value
  integer :: i, num_pairs
  type(ParamEntry) :: entry
  @@ -844,7 +844,7 @@ module paramreader_module
  character(len=*), optional :: key
  logical :: status
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS) :: fields
• character(len=STRING_LENGTH), dimension(MAX_N_FIELDS), save :: fields
  integer :: num_fields, j

if (entry%param_type == PARAM_NO_VALUE .or. &
—
Reply to this email directly or view it on GitHub <
https://github.com/libAtoms/QUIP/issues/8#issuecomment-174960155>.

—
Reply to this email directly or view it on GitHub
#8 (comment).