Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

duk_config.h increase RECLIMITs #58

Open
wants to merge 2 commits into
base: v3
Choose a base branch
from
Open

duk_config.h increase RECLIMITs #58

wants to merge 2 commits into from

Conversation

linuxion
Copy link

Hi, @olebedev

As described in this PR when geth calls JsonEncode function from your library it fails in case of reaching the DUK_USE_JSON_ENC_RECLIMIT = 1000.

So what is the purpose of setting the reclimits of 1000? Are there any risks in increasing them?

Thank you!

@olebedev
Copy link
Owner

Hi @linuxion,

I think we have to ask @svaarala what was the purpose of it.

@svaarala
Copy link

There's no risk if the platform provides enough stack. The default limit is a compromise so that on most platforms the limit is reached before one actually runs out of stack (which often leads to a segfault or "abort").

So depending on the platform, one would first ensure there's enough stack if the platform default is too low, and then use Duktape normally with the RECLIMIT increased.

@linuxion
Copy link
Author

Thank you @svaarala,

@olebedev what are you going to do with this? The issue is that your library is vendored in go-ethereum so devs can't just change the code.

@olebedev
Copy link
Owner

olebedev commented Apr 13, 2018

What platforms do you target with go-ethereum?

@karalabe
Copy link
Contributor

Duktape is only used for transaction tracing, so it currently is only used on fairly beefy machines.

Would it be possible somehow to have this configurable via Go, so that you wouldn't need to bump the limits for everybody but still allow "power users" to run with higher limits without having to fork?

@holiman
Copy link

holiman commented Feb 7, 2019

Any update on this? We would prefer to be able to use a vendored dependency of duktape instead of other hacks around it.

@bulgakovk
Copy link

Would also love If that could be reviewed and merged

@bulgakovk
Copy link

Hey any update on this?

@peterwillcn
Copy link

duk_logging.c: In function ‘duk__logger_prototype_log_shared’:
duk_logging.c:184:64: warning: ‘Z’ directive writing 1 byte into a region of size between 0 and 9 [-Wformat-overflow=]
  184 |  sprintf((char *) date_buf, "%04d-%02d-%02dT%02d:%02d:%02d.%03dZ",
      |                                                                ^
In file included from /usr/include/stdio.h:867,
                 from duk_logging.c:5:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:36:10: note: ‘__builtin___sprintf_chk’ output between 25 and 85 bytes into a destination of size 32
   36 |   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   37 |       __bos (__s), __fmt, __va_arg_pack ());
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@tanmaster
Copy link

Any update on this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants